Re: [PATCH v2 07/14] xen/riscv: introduce exception context

[Jan Beulich <jbeulich@xxxxxxxx>]

On 30.01.2023 12:54, Oleksii wrote:
> Hi Jan,
> 
> On Fri, 2023-01-27 at 15:24 +0100, Jan Beulich wrote:
>> On 27.01.2023 14:59, Oleksii Kurochko wrote:
>>> --- /dev/null
>>> +++ b/xen/arch/riscv/include/asm/processor.h
>>> @@ -0,0 +1,82 @@
>>> +/* SPDX-License-Identifier: MIT */
>>> +/*****************************************************************
>>> *************
>>> + *
>>> + * Copyright 2019 (C) Alistair Francis <alistair.francis@xxxxxxx>
>>> + * Copyright 2021 (C) Bobby Eshleman <bobby.eshleman@xxxxxxxxx>
>>> + * Copyright 2023 (C) Vates
>>> + *
>>> + */
>>> +
>>> +#ifndef _ASM_RISCV_PROCESSOR_H
>>> +#define _ASM_RISCV_PROCESSOR_H
>>> +
>>> +#ifndef __ASSEMBLY__
>>> +
>>> +/* On stack VCPU state */
>>> +struct cpu_user_regs
>>> +{
>>> +    unsigned long zero;
>>> +    unsigned long ra;
>>> +    unsigned long sp;
>>> +    unsigned long gp;
>>> +    unsigned long tp;
>>> +    unsigned long t0;
>>> +    unsigned long t1;
>>> +    unsigned long t2;
>>> +    unsigned long s0;
>>> +    unsigned long s1;
>>> +    unsigned long a0;
>>> +    unsigned long a1;
>>> +    unsigned long a2;
>>> +    unsigned long a3;
>>> +    unsigned long a4;
>>> +    unsigned long a5;
>>> +    unsigned long a6;
>>> +    unsigned long a7;
>>> +    unsigned long s2;
>>> +    unsigned long s3;
>>> +    unsigned long s4;
>>> +    unsigned long s5;
>>> +    unsigned long s6;
>>> +    unsigned long s7;
>>> +    unsigned long s8;
>>> +    unsigned long s9;
>>> +    unsigned long s10;
>>> +    unsigned long s11;
>>> +    unsigned long t3;
>>> +    unsigned long t4;
>>> +    unsigned long t5;
>>> +    unsigned long t6;
>>> +    unsigned long sepc;
>>> +    unsigned long sstatus;
>>> +    /* pointer to previous stack_cpu_regs */
>>> +    unsigned long pregs;
>>> +};
>>
>> Just to restate what I said on the earlier version: We have a struct
>> of
>> this name in the public interface for x86. Besides to confusion about
>> re-using the name for something private, I'd still like to understand
>> what the public interface plans are. This is specifically because I
>> think it would be better to re-use suitable public interface structs
>> internally where possible. But that of course requires spelling out
>> such parts of the public interface first.
>>
> I am not sure that I get you here.
> I greped a little bit and found out that each architecture declares
> this structure inside arch-specific folder.
> 
> Mostly the usage of the cpu_user_regs is to save/restore current state
> of CPU during traps ( exceptions/interrupts ) and context_switch().

Arm effectively duplicates the public interface struct vcpu_guest_core_regs
and the internal struct cpu_user_regs (and this goes as far as also
duplicating the __DECL_REG() helper). Personally I find such duplication
odd at the first glance at least; maybe there is a specific reason for this
in Arm. But whether the public interface struct can be re-used can likely
only be known once it was spelled out.

> Also some registers are modified during construction of a domain.
> Thereby I prefer here to see the arch-specific register names instead
> of common.

Not sure what meaning of "common" you imply here. Surely register names
want to be arch-specific, and hence can't be "common" with other arch-es.

Jan