Hi, > On 17 Oct 2022, at 10:17, Bertrand Marquis <[email protected]> wrote: > > Hi, > >> On 15 Oct 2022, at 10:28, Julien Grall <[email protected]> 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
