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

[Ayan Kumar Halder <ayankuma@xxxxxxx>]

On 13/10/2022 16:38, Julien Grall wrote:
> Hi,
Hi Julien,
> On 13/10/2022 16:13, Ayan Kumar Halder wrote:
> > On 13/10/2022 15:47, Julien Grall wrote:
> > > > .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.
> Why not? Your GICR_TYPER is a 64-bit and the helper is the right way 
> to hide whether the low/high 32-bit are accessed.
When Zephyr issues 32 bit reads for GICR_TYPER for GICR_TYPER + 4, it 
traps to Xen two times.
So, my understanding is that Xen needs to read two 32 bit values.

We can modify the vreg_reg64_extract() so that it can return the lower 
32 bits in one case and upper 32 bits in the second case.
Is this what you are suggesting ?

- Ayan

> > 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. |
> This is a bit difficult to read. So I looked at your answer on the 
> Zephyr thread.
> You will end up to duplicate code for emulating TYPER and there are 
> quite a few other use of vreg_reg64_extract() in the code.
> So I am afraid this is not an option for me. If you implement 
> vreg_reg64_* then you will not need to implement.
> I would be open to other suggestion so long we don't end up with 
> duplication or any other kind of open-coding.
> Cheers,