Re: Need guidance to support reading GICR_TYPER (64 bit register) on Aarch32_v8r

[Bertrand Marquis <Bertrand.Marquis@xxxxxxx>]

Hi,

> On 17 Oct 2022, at 10:17, Bertrand Marquis <Bertrand.Marquis@xxxxxxx> wrote:
> 
> Hi,
> 
>> On 15 Oct 2022, at 10:28, Julien Grall <julien@xxxxxxx> wrote:
>> 
>> Hi Stefano,
>> 
>> On 14/10/2022 19:09, Stefano Stabellini wrote:
>>> On Thu, 13 Oct 2022, Ayan Kumar Halder wrote:
>>>> Hi Arm mantainers/Folks,
>>>> 
>>>> Please refer to the discussion
>>>> https://github.com/zephyrproject-rtos/zephyr/pull/51163 .
>>>> 
>>>> We intend to run Zephyr as a domU guest on Xen on Aarch32_v8R fixed virtual
>>>> platform.
>>>> 
>>>> Zephyr is trying to read GICR_TYPER which is a 64 bit register using ldrd
>>>> instruction.
>>>> 
>>>> As GICR is emulated by Xen, so this instruction gets trapped with HSR =
>>>> 0x9200000c.
>>>> 
>>>> As ISV is 0, so Xen cannot emulate this instruction.
>>>> 
>>>> The proposed solution is to use two sys_read32() on GICR_TYPER to return 
>>>> the
>>>> lower and upper 32 bits.
>>>> 
>>>> With this, HSR = 0x9383 000c, ISV=1 so ISS is valid.
>>> Hi all,
>>> I wanted to take a step back on this issue before we jump into the
>>> details.
>>> Differently from other instructions we discussed in the past, strd and ldrd
>>> are not deprecated and are not "unusual corner cases". There is no
>>> statements such as "please don't use this" on the ARM ARM. If I were to
>>> write an register read/write function in assembly for an RTOS, it would
>>> be reasonable to use them.
>> 
>> Just to be clear it is fine to use the ldrd/strd for accessing non MMIO 
>> area. The problem comes with MMIO access because they can be emulated by the 
>> hypervisor and we don't have the syndrome. At the moment, this is only a 
>> problem when accessing some of the GICv3 (including ITS) registers.
>> 
>>> So, I struggle to see how we'll be able to deal with all the possible
>>> RTOSes out there that might have them in the code. We can fix Zephyr,
>>> but what about FreeRTOS, ThreadX and the proprietary ones (VxWorks,
>>> etc.)?
>> 
>> This is not an Xen issue but architecture issue. The RTOSes will face the 
>> exact same issue on any hypervisors unless they decided to decode the 
>> instruction.
>> 
>> As we discussed before decoding an instruction correctly is quite difficult 
>> to do (what we have in Xen for pos-increment store/load is just a band-aid). 
>> So I would expect the other hypervisors to have made the decision to not 
>> implement it. AFAIK KVM doesn't suppor them,
>> Note that looking at ID_ISAR2, it seems that ldrd/strd is technically 
>> optional. Therefore, the RTOS would have to assume it is targeting a 
>> processor that supports them.
>> 
>>> Unless we can get ARM to issue a clear guidance that strd and ldrd are
>>> deprecated, 
>> 
>> Arm Arm cannot say that because ldrd/strd are necessary to modify the LPAE 
>> page-tables atomically. What we need to know is which instructions can be 
>> allowed on MMIO accesses.
> 
> Definitely this is something that arm arm cannot fully answer as it is also 
> down to the full platform. MMIO accesses are going out of the CPU and hence 
> wether or not 64bit MMIO accesses can be properly done might also depend on 
> the bus or the IP on the other side (some peripherals might just refuse 64bit 
> accesses or some bus might only be 32bit so the operations would need to be 
> divided).
> 
>> 
>> I think I already raised that when Ayan added decoding for post-increment 
>> instructions. There are plenty of instructions (or combinations) that 
>> doesn't provide a syndrome and yet the processor doesn't prevent anyone to 
>> use them on MMIO.
>> 
>> I was worry we are going to have to continue to decode instructions in a 
>> non-compliant way in Xen just to please a few RTOs that may not even run 
>> anywhere else.
>> 
>> This would also reduce our leverage to request a change in the RTOes or the 
>> Arm Arm (maybe there is already a statement I haven't spotted) because Xen 
>> will already (badly) support the instruction.
> 
> Going back on the ID_ISAR2, if Xen is properly setting the value seen by the 
> guests, there is not reason for us to actually emulate those instructions.
> The emulation code inside Xen cost a lot in matter of lines of code and would 
> need a lot of testing (which is missing at the moment).
> So as we have a standard way to inform the guest that this is not supported, 
> we should stick to that.
> 
>> 
>>> I think it would be better to attempt to decode them rather
>>> than just fail. I don't like to have this kind of code in Xen, but I
>>> don't see a way to support R52s without it.
>> That's not specific to R52. This is anyone using GICv3 on Arm32 core.
> 
> Agree.
> 
>> 
>>> That said, of course if Zephyr was to use two 32-bit reads instead of
>>> one 64-bit read, it would be better for Xen. And we have more important
>>> things to deal with right now in terms of R52 support (it is not even
>>> upstream yet). So it is totally fine to change Zephyr and move forward
>>> for now.
>>> But medium term it doesn't seem to me that we can get away without a
>>> solution in Xen for this (or a change in the ARM ARM).
>> 
>> See above. This is an architecture problem and we should discuss with Arm 
>> first before continuing to add more decoding in Xen.
> 
> I will discuss it internally to have an answer but I think that the answer 
> cannot only come from Arm as there are for sure hardware implementations that 
> cannot support this, as explain before.

I had some discussions internally and here is the official view:

>From the architecture point of view this should always work but this is not 
>virtualisable (as there is no syndrome register) and not recommended as 
>deferencing a pointer accessing MMIO registers is not safe, so it should not 
>be done for MMIO.

Linux is not doing those kind of accesses and KVM does not support guest doing 
them.

So I think we should not try to emulate this.

Cheers
Bertrand


> 
> Cheers
> Bertrand
> 
>> 
>> Cheers,
>> 
>> -- 
>> Julien Grall