HI Volodymyr,
> On 1 Dec 2020, at 16:54, Volodymyr Babchuk <volodymyr_babc...@epam.com> wrote:
>
>
> Hi,
>
> Bertrand Marquis writes:
>
>> Hi Volodymyr,
>>
>>> On 1 Dec 2020, at 12:07, Volodymyr Babchuk <volodymyr_babc...@epam.com>
>>> wrote:
>>>
>>>
>>> Hi,
>>>
>>> Bertrand Marquis writes:
>>>
>>>> Hi,
>>>>
>>>>> On 30 Nov 2020, at 20:31, Volodymyr Babchuk <volodymyr_babc...@epam.com>
>>>>> wrote:
>>>>>
>>>>>
>>>>> Bertrand Marquis writes:
>>>>>
>>>>>> Add support for emulation of cp15 based ID registers (on arm32 or when
>>>>>> running a 32bit guest on arm64).
>>>>>> The handlers are returning the values stored in the guest_cpuinfo
>>>>>> structure.
>>>>>> In the current status the MVFR registers are no supported.
>>>>>
>>>>> It is unclear what will happen with registers that are not covered by
>>>>> guest_cpuinfo structure. According to ARM ARM, it is implementation
>>>>> defined if such accesses will be trapped. On other hand, there are many
>>>>> registers which are RAZ. So, good behaving guest can try to read one of
>>>>> that registers and it will get undefined instruction exception, instead
>>>>> of just reading all zeroes.
>>>>
>>>> This is true in the status of this patch but this is solved by the next
>>>> patch
>>>> which is adding proper handling of those registers (add CP10 exception
>>>> support), at least for MVFR ones.
>>>>
>>>> From ARM ARM point of view, I did handle all registers listed I think.
>>>> If you think some are missing please point me to them as O do not
>>>> completely understand what are the “registers not covered” unless
>>>> you mean the MVFR ones.
>>>
>>> Well, I may be wrong for aarch32 case, but for aarch64, there are number
>>> of reserved registers in IDs range. Those registers should read as
>>> zero. You can find them in the section "C5.1.6 op0==0b11, Moves to and
>>> from non-debug System registers and Special-purpose registers" of ARM
>>> DDI 0487B.a. Check out "Table C5-6 System instruction encodings for
>>> non-Debug System register accesses".
>>
>> The point of the serie is to handle all registers trapped due to TID3 bit in
>> HCR_EL2.
>>
>> And i think I handled all of them but I might be wrong.
>>
>> Handling all registers for op0==0b11 will cover a lot more things.
>> This can be done of course but this was not the point of this serie.
>>
>> The listing in HCR_EL2 documentation is pretty complete and if I miss any
>> register
>> there please tell me but I do no understand from the documentation that all
>> registers
>> with op0 3 are trapped by TID3.
>
> My concern is that the same code may observe different effects when
> running in baremetal mode and as VM.
>
> For example, we are trying to run a newer version of a kernel, that
> supports some hypothetical ARMv8.9. And it tries to read a new ID
> register which is absent in ARMv8.2. There are possible cases:
>
> 0. It runs as a baremetal code on a compatible architecture. So it just
> accesses this register and all is fine.
>
> 1. It runs as baremetal code on older ARM8 architecture. Current
> reference manual states that those registers should read as zero, so
> all is fine, as well.
>
> 2. It runs as VM on an older architecture. It is IMPLEMENTATION DEFINED
> if HCR.ID3 will cause traps when access to unassigned registers:
>
> 2a. Platform does not cause traps and software reads zeros directly from
> real registers. This is a good outcome.
>
> 2b. Platform causes trap, and your code injects the undefined
> instruction exception. This is a case that bothers me. Well written code
> that is tries to be compatible with different versions of architecture
> will fail there.
>
> 3. It runs as VM on a never architecture. I can only speculate there,
> but I think, that list of registers trapped by HCR.ID3 will be extended
> when new features are added. At least, this does not contradict with the
> current IMPLEMENTATION DEFINED constraint. In this case, hypervisor will
> inject exception when guest tries to access a valid register.
>
>
> So, in my opinion and to be compatible with the reference manual, we
> should allow guests to read "Reserved, RAZ" registers.
Thanks for the very detailed explanation, I know better understand what you
mean here.
I will try to check if we could return RAZ for “other” op0=3 registers and what
should be done on cp15 registers to have something equivalent.
Regards
Bertrand
>
>
>
>> Regards
>> Bertrand
>>
>>
>>>
>>>
>>>>>
>>>>>> Signed-off-by: Bertrand Marquis <bertrand.marq...@arm.com>
>>>>>> ---
>>>>>> Changes in V2: rebase
>>>>>> ---
>>>>>> xen/arch/arm/vcpreg.c | 35 +++++++++++++++++++++++++++++++++++
>>>>>> 1 file changed, 35 insertions(+)
>>>>>>
>>>>>> diff --git a/xen/arch/arm/vcpreg.c b/xen/arch/arm/vcpreg.c
>>>>>> index cdc91cdf5b..d0c6406f34 100644
>>>>>> --- a/xen/arch/arm/vcpreg.c
>>>>>> +++ b/xen/arch/arm/vcpreg.c
>>>>>> @@ -155,6 +155,14 @@ TVM_REG32(CONTEXTIDR, CONTEXTIDR_EL1)
>>>>>> break; \
>>>>>> }
>>>>>>
>>>>>> +/* Macro to generate easily case for ID co-processor emulation */
>>>>>> +#define GENERATE_TID3_INFO(reg,field,offset) \
>>>>>> + case HSR_CPREG32(reg): \
>>>>>> + { \
>>>>>> + return handle_ro_read_val(regs, regidx, cp32.read, hsr, \
>>>>>> + 1, guest_cpuinfo.field.bits[offset]); \
>>>>>> + }
>>>>>> +
>>>>>> void do_cp15_32(struct cpu_user_regs *regs, const union hsr hsr)
>>>>>> {
>>>>>> const struct hsr_cp32 cp32 = hsr.cp32;
>>>>>> @@ -286,6 +294,33 @@ void do_cp15_32(struct cpu_user_regs *regs, const
>>>>>> union hsr hsr)
>>>>>> */
>>>>>> return handle_raz_wi(regs, regidx, cp32.read, hsr, 1);
>>>>>>
>>>>>> + /*
>>>>>> + * HCR_EL2.TID3
>>>>>> + *
>>>>>> + * This is trapping most Identification registers used by a guest
>>>>>> + * to identify the processor features
>>>>>> + */
>>>>>> + GENERATE_TID3_INFO(ID_PFR0, pfr32, 0)
>>>>>> + GENERATE_TID3_INFO(ID_PFR1, pfr32, 1)
>>>>>> + GENERATE_TID3_INFO(ID_PFR2, pfr32, 2)
>>>>>> + GENERATE_TID3_INFO(ID_DFR0, dbg32, 0)
>>>>>> + GENERATE_TID3_INFO(ID_DFR1, dbg32, 1)
>>>>>> + GENERATE_TID3_INFO(ID_AFR0, aux32, 0)
>>>>>> + GENERATE_TID3_INFO(ID_MMFR0, mm32, 0)
>>>>>> + GENERATE_TID3_INFO(ID_MMFR1, mm32, 1)
>>>>>> + GENERATE_TID3_INFO(ID_MMFR2, mm32, 2)
>>>>>> + GENERATE_TID3_INFO(ID_MMFR3, mm32, 3)
>>>>>> + GENERATE_TID3_INFO(ID_MMFR4, mm32, 4)
>>>>>> + GENERATE_TID3_INFO(ID_MMFR5, mm32, 5)
>>>>>> + GENERATE_TID3_INFO(ID_ISAR0, isa32, 0)
>>>>>> + GENERATE_TID3_INFO(ID_ISAR1, isa32, 1)
>>>>>> + GENERATE_TID3_INFO(ID_ISAR2, isa32, 2)
>>>>>> + GENERATE_TID3_INFO(ID_ISAR3, isa32, 3)
>>>>>> + GENERATE_TID3_INFO(ID_ISAR4, isa32, 4)
>>>>>> + GENERATE_TID3_INFO(ID_ISAR5, isa32, 5)
>>>>>> + GENERATE_TID3_INFO(ID_ISAR6, isa32, 6)
>>>>>> + /* MVFR registers are in cp10 no cp15 */
>>>>>> +
>>>>>> /*
>>>>>> * HCR_EL2.TIDCP
>>>>>> *
>>>>>
>>>>>
>>>>> --
>>>>> Volodymyr Babchuk at EPAM
>>>
>>>
>>> --
>>> Volodymyr Babchuk at EPAM
>
>
> --
> Volodymyr Babchuk at EPAM
