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

[Bertrand Marquis <Bertrand.Marquis@xxxxxxx>]

> 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.

We need those instructions for page table access and it is mandatory to support 
that on armv8, so not the solution definitely.

> 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.
> 
> Cheers
> Bertrand
> 
>> 
>> Cheers,
>> 
>> -- 
>> Julien Grall