Re: [PATCH v1 03/15] xen/riscv: implement vcpu_csr_init()

[Jan Beulich <jbeulich@xxxxxxxx>]

On 12.01.2026 17:39, Oleksii Kurochko wrote:
> 
> On 1/12/26 4:54 PM, Jan Beulich wrote:
>> On 12.01.2026 16:46, Oleksii Kurochko wrote:
>>> On 1/12/26 3:28 PM, Jan Beulich wrote:
>>>> On 12.01.2026 13:59, Oleksii Kurochko wrote:
>>>>> On 1/7/26 9:46 AM, Jan Beulich wrote:
>>>>>> Also, wouldn't you better keep internal state in line with what hardware
>>>>>> actually supports? CSRIND may be read-only-zero in the real register, in
>>>>>> which case having the bit set in the "cached" copy can be misleading.
>>>>> [...]
>>>>>
>>>>>> (This may similarly apply to at least hedeleg and hideleg, btw.)
>>>>> Regarding the previous bits, I can understand that it would be an issue:
>>>>> if SSAIA isn’t supported, then it is incorrect to update the corresponding
>>>>> bits of|hstateen0|.
>>>>>
>>>>> However, I’m not really sure I understand what the issue is 
>>>>> with|h{i,e}deleg|.
>>>>> All writable bits there don’t depend on hardware support. Am I missing 
>>>>> something?
>>>> My reading of the doc was that any of the bits can be r/o 0, with - yes -
>>>> no dependencies on particular extensions.
>>> Just to be sure that I get your idea correctly.
>>>
>>> Based on the priv. spec:
>>>     Each bit of hedeleg shall be either writable or read-only zero. Many 
>>> bits of
>>>     hedeleg are required specifically to be writable or zero, as enumerated 
>>> in
>>>     Table 29.
>>>
>>> Now let’s take hedeleg.bit1, which is marked as writable according to Table 
>>> 29.
>>> Your point is that even though hedeleg.bit1 is defined as writable, it 
>>> could still
>>> be read-only zero, right?
>>>
>>> In general, I agree with that. It is possible that M-mode software decides, 
>>> for
>>> some reason (for example, because the implementation does not support 
>>> delegation
>>> of bit1 to a lower mode), not to delegate medeleg.bit1 to HS-mode. In that 
>>> case,
>>> hedeleg.bit1 would always be read-only zero.
>>>
>>>>    In which case you'd need to do
>>>> the delegation in software. For which it might be helpful to know what
>>>> the two registers are actually set to in hardware (i.e. the cached values
>>>> wanting to match the real ones).
>>> Does it make sense then to have the following
>>>             ...
>>>     v->arch.hedeleg = hedeleg;
>>>             vcpu->arch.hedeleg = csr_read(CSR_HEDELEG);
>>> in arch_vcpu_create()?
>> The above makes no sense to me, with or without s/vcpu/v/.
> 
> Right...
> 
> It should be also csr_write() before csr_read():
>   csr_write(CSR_HEDELEG, hedeleg);
>   v->arch.hedeleg = csr_read(CSR_HEDELEG);

Ah yes. Alternatively you could obtain a mask of modifiable bits once, and
then simply apply that here in place of the CSR read/write.

Jan