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

[Jan Beulich <jbeulich@xxxxxxxx>]

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

> Or I can just add the comment that it will be sync-ed with the corresponding
> hardware CSR later as ,actually, there is some h{i,e}deleg synchronization
> happening during context_switch() (this code is at the moment in downstream),
> because restore_csr_regs() is executed and re-reads CSR_H{I,E}DELEG:
>    static void restore_csr_regs(struct vcpu *vcpu)
>    {
>        csr_write(CSR_HEDELEG, vcpu->arch.hedeleg);
>        csr_write(CSR_HIDELEG, vcpu->arch.hideleg);
>        ...
> As a result, vcpu->arch.h{I,E}deleg is kept in sync with the corresponding
> hardware CSR.

No, the r/o bits will continue to be out-of-sync between the hw register and
the struct arch_vcpu field.

Jan