Re: [PATCH v2 03/16] xen/riscv: implement vcpu_csr_init()

[Jan Beulich <jbeulich@xxxxxxxx>]

On 24.01.2026 23:44, Teddy Astie wrote:
> Le 22/01/2026 à 17:49, Oleksii Kurochko a écrit :
>> +static void vcpu_csr_init(struct vcpu *v)
>> +{
>> +    register_t hstateen0;
>> +
>> +    csr_write(CSR_HEDELEG, HEDELEG_DEFAULT);
>> +    v->arch.hedeleg = csr_read(CSR_HEDELEG);
>> +
>> +    vcpu_guest_cpu_user_regs(v)->hstatus = HSTATUS_SPV | HSTATUS_SPVP;
>> +
>> +    csr_write(CSR_HIDELEG, HIDELEG_DEFAULT);
>> +    v->arch.hideleg = csr_read(CSR_HIDELEG);
>> +
> 
> To me, that feels odd to set machine CSR here. Especially if (I guess) 
> that we would update them anyway during context switches.
> 
> I think it would be better to have :
> - vcpu_csr_init -> sets initial state CSR in vcpu structure, doesn't 
> touch machine CSR
> - context switching logic -> loads vcpu-specific machine CSR from vcpu 
> structure
> 
> We would have to make a context switch to enter the vcpu for the first 
> time; but that's to be expected.
> 
> With my proposal, we would also want to move the vcpu_csr_init() 
> invocation to the place we initialize the vcpu_arch structure rather 
> than the first time we schedule inside that vcpu.

Aiui the writes were put here to be able to cope with r/o-zero bits. Yet
indeed it can't be cone like this. While it may work for domains created
during boot, these CSRs would be changed under the feet of a running vCPU
when done this way for domain creation later at runtime.

Instead, as I think I had also suggested earlier on, the r/o-zero-ness of
bits in particular CSRs wants determining once during boot, and then that
mask wants using when setting up vCPU-s.

Jan