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

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Mon, 17 Oct 2022, Bertrand Marquis wrote:
> > On 17 Oct 2022, at 10:17, Bertrand Marquis <Bertrand.Marquis@xxxxxxx> wrote:
> >> On 15 Oct 2022, at 10:28, Julien Grall <julien@xxxxxxx> wrote:
> >> 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.

I wouldn't take Linux and KVM as role models for the embedded space.
Zephyr would be better (unfortunately Zephyr is not behaving as we
prefer today).


At least at AMD/Xilinx, our users try something on native first, then
they try it on Xen, and if it doesn't work they blame Xen. They don't
typically try KVM and compare behavior. We try to explain that Xen is
not necessarily to blame but this is the natural way of thinking for
engineers apparently as it happened many times in the last few years for
a range of issues with users in very different
geographys/companies/projects.

I am writing the above just for context and to create a common
understanding of Xen users' behavior and way of thinking. I don't think
we can succeed in changing our users' way of thinking.

So generally my preference is to try to prevent a situation where a user
might reach out to us in regard to something that used to work on native
and doesn't anymore on Xen. It is always very difficult to explain in a
way that is convincing to the user.


But honestly, we had zero reports of ldrd/strd causing a guest crash
from users so far. So I think it is OK not to introduce code in the
hypervisor to fix a "theoretical" bug. So one approach is to wait for
the first real user report of the problem and then re-discuss the issue
at that point in time (if it ever happens).

Documentation would help if we had it -- we don't have an official
document anywhere stating that Xen as a project doesn't intend to
support ltrd/strd on MMIO regions. That would help.

If we had such a document, it would be easier to explain the issue to
users and they would more easily fix their RTOS if they can. There is
also the possibility that they can't, because it is provided by
third-parties. Like above, if it happens we can re-discuss it at that
point in time.