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

[Julien Grall <julien@xxxxxxx>]

On 17/10/2022 10:17, Bertrand Marquis wrote:
> Hi,
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.
Thanks for the feedback. AFAIU, the bit in ID_ISAR2 indicates whether 
ldrd/strd is present. If we decide to clear the bit, then it would mean 
the guest should not use them even when modifying LPAE page-tables.
This could be a problem because AFAIK those instructions are necessary 
to modify the page-tables atomically. Therefore, I don't Xen should 
clear the bit.
Cheers,

--
Julien Grall