Hi Stefano,
> On 10 Dec 2020, at 22:29, Stefano Stabellini <sstabell...@kernel.org> wrote:
>
> On Thu, 10 Dec 2020, Bertrand Marquis wrote:
>> Hi Stefano,
>>
>>> On 9 Dec 2020, at 19:38, Stefano Stabellini <sstabell...@kernel.org> wrote:
>>>
>>> On Wed, 9 Dec 2020, Bertrand Marquis wrote:
>>>> Add vsysreg emulation for registers trapped when TID3 bit is activated
>>>> in HSR.
>>>> The emulation is returning the value stored in cpuinfo_guest structure
>>>> for know registers and is handling reserved registers as RAZ.
>>>>
>>>> Signed-off-by: Bertrand Marquis <bertrand.marq...@arm.com>
>>>> ---
>>>> Changes in V2: Rebase
>>>> Changes in V3:
>>>> Fix commit message
>>>> Fix code style for GENERATE_TID3_INFO declaration
>>>> Add handling of reserved registers as RAZ.
>>>>
>>>> ---
>>>> xen/arch/arm/arm64/vsysreg.c | 53 ++++++++++++++++++++++++++++++++++++
>>>> 1 file changed, 53 insertions(+)
>>>>
>>>> diff --git a/xen/arch/arm/arm64/vsysreg.c b/xen/arch/arm/arm64/vsysreg.c
>>>> index 8a85507d9d..ef7a11dbdd 100644
>>>> --- a/xen/arch/arm/arm64/vsysreg.c
>>>> +++ b/xen/arch/arm/arm64/vsysreg.c
>>>> @@ -69,6 +69,14 @@ TVM_REG(CONTEXTIDR_EL1)
>>>> break; \
>>>> }
>>>>
>>>> +/* Macro to generate easily case for ID co-processor emulation */
>>>> +#define GENERATE_TID3_INFO(reg, field, offset) \
>>>> + case HSR_SYSREG_##reg: \
>>>> + { \
>>>> + return handle_ro_read_val(regs, regidx, hsr.sysreg.read, hsr, \
>>>> + 1, guest_cpuinfo.field.bits[offset]); \
>>>
>>> [...]
>>>
>>>> + HSR_SYSREG_TID3_RESERVED_CASE:
>>>> + /* Handle all reserved registers as RAZ */
>>>> + return handle_ro_raz(regs, regidx, hsr.sysreg.read, hsr, 1);
>>>
>>>
>>> We are implementing both the known and the implementation defined
>>> registers as read-as-zero. On write, we inject an exception.
>>>
>>> However, reading the manual, it looks like the implementation defined
>>> registers should be read-as-zero/write-ignore, is that right?
>>
>> In the documentation, I did find all those defined as RO (Arm Architecture
>> reference manual, chapter D12.3.1). Do you think we should handle Read
>> only register as write ignore ? now i think of it RO does not explicitely
>> mean
>> if writes are ignored or should generate an exception.
>>
>>>
>>> I couldn't easily find in the manual if it is OK to inject an exception
>>> on write to a known register.
>>
>> I am actually unsure if it should or not.
>> I will try to run a test to check what is happening if this is done on the
>> real hardware and come back to you on this one.
>
> Yeah, that's the best way to do it: if writes are ignored on real
> hardware, let's turn this into read-only/write-ignore, otherwise if they
> generate an exception then let's keep the code as is.
>
> Also you might want to do that both for a known register and also for an
> unknown register to see if it makes a difference.
I did a test with the following:
- WRITE_SYSREG64(0xf, S3_0_C0_C3_3)
- WRITE_SYSREG64(0xf, ID_MMFR0_EL1)
- WRITE_SYSREG64(0xf, ID_AA64MMFR0_EL1)
All generate exceptions like:
Hypervisor Trap. HSR=0x2000000 EC=0x0 IL=1 Syndrome=0x0
So I think it is right to generate an exception if one of them is accessed.
Regards
Bertrand
>
> Thank you!
