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

[Ayan Kumar Halder <ayankuma@xxxxxxx>]

On 13/10/2022 15:47, Julien Grall wrote:
> Hi Ayan,
Hi Julien,
> On 13/10/2022 15:30, Ayan Kumar Halder wrote:
> > On 13/10/2022 14:20, Julien Grall wrote:
> > > On 13/10/2022 12:47, Ayan Kumar Halder wrote:
> > > > Hi Arm mantainers/Folks,
> > > Hello,
> > Hi Julien,
> >
> > Appreciate your help as always. :)
> >
> > > > 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.
> > > >
> > > > Now, for Xen to emulate this read, I have proposed the 
> > > > modifications (in my last comment).
> > > I am confused. Looking at the emulation of GICR_TYPER in Xen 
> > > (arch/arm/vgic-v3.c), the code should already be able to handle 
> > > 32-bit access.
> > When I compile Xen for arm32, vreg_reg64_extract() definitions do not 
> > exist.
> Ah. You are building Xen for arm32... This is unsupported by Xen at 
> the moment.
Yes.
> > The reason being 
> > https://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=xen/arch/arm/include/asm/vreg.h;h=f26a70d024e2d1f933ea66793ea6e42f81c7a8cf;hb=9029bc265cdf2bd63376dde9fdd91db4ce9c0586#l189 
> > , BITS_PER_LONG is 32 (Refer xen/arch/arm/include/asm/config.h).
> Right, so that's not an architecture issue (like 'ldrd') but just Xen 
> not providing the helper.
Sorry, I was not very clear.

With ldrd, HSR=0x9200000c. As ISV=0, Xen cannot emulate the instruction.

So even if Xen had implemented vreg_reg64_extract() for arm32 GICv3, it 
would not have helped here. Unless, we decide to decode the instruction 
which may not be correct thing to do (as you explained in the comment on 
https://github.com/zephyrproject-rtos/zephyr/pull/51163).
> As I wrote above, Xen doesn't yet support GICv3 on arm32. 
> I expect that implementing vreg_reg64_extract() will be one of the step.
I don't think it is required in this particular scenario. I have 
explained below.
> > .Thus, the guest need to invoke sys_read32() twice (GICR_TYPER and 
> > GICR_TYPER+4).
> I don't understand how you came to this conclusion with what you wrote. 
Sorry for confusion. I was trying to explain that the code does not 
handle 32 bit access of GICR_TYPER on arm32.

> If we had implemented vreg_reg64_extract(), then Zephyr would still 
> need to issue two 32-bit read because Xen doesn't emulate 'ldrd'.
Yes. So what we need here is not the implementation of 
vreg_reg64_extract() for GICR_TYPER.
Zephyr will issue two 32 bit read access for GICR_TYPER and (GICR_TYPER 
+ 4). So Xen needs to emulate 32 bit read for GICR_TYPER and GICR_TYPER + 4.
So, we will need to modify the Xen code as follows :-

|#ifdef AARCH32 case VREG32(GICR_TYPER_L32) : { read and return lower 32 
bits of GICR_TYPER return 1; } case VREG32(GICR_TYPER_H32) : { read and 
return the upper 32 bits of GICR_TYPER return 1; } #else /* For AARCH64 
*/ case VREG64(GICR_TYPER): { do as existing; } #endif Let me know if 
this looks sane. I can then submit a single patch to add support for this. |
> Cheers,