Re: [PATCH v6 01/12] xen/arm: enable SVE extension for Xen

[Julien Grall <julien@xxxxxxx>]

Hi,

On 19/05/2023 16:13, Luca Fancellu wrote:
> > On 19/05/2023 15:51, Luca Fancellu wrote:
> > > /* Control Registers */
> > > /*
> > > * CPTR_EL2 needs to be written before calling vfp_restore_state, a
> > > * synchronization instruction is expected after the write (isb)
> > > */
> > > WRITE_SYSREG(n->arch.cptr_el2, CPTR_EL2);
> > > WRITE_SYSREG(n->arch.cpacr, CPACR_EL1);
> > > /*
> > > * This write to sysreg CONTEXTIDR_EL1 ensures we don't hit erratum
> > > * #852523 (Cortex-A57) or #853709 (Cortex-A72).
> > > * I.e DACR32_EL2 is not correctly synchronized.
> > > */
> > > WRITE_SYSREG(n->arch.contextidr, CONTEXTIDR_EL1);
> > > WRITE_SYSREG(n->arch.tpidr_el0, TPIDR_EL0);
> > > WRITE_SYSREG(n->arch.tpidrro_el0, TPIDRRO_EL0);
> > > WRITE_SYSREG(n->arch.tpidr_el1, TPIDR_EL1);
> > > if ( is_32bit_domain(n->domain) && cpu_has_thumbee )
> > > {
> > > WRITE_SYSREG(n->arch.teecr, TEECR32_EL1);
> > > WRITE_SYSREG(n->arch.teehbr, TEEHBR32_EL1);
> > > }
> > > #ifdef CONFIG_ARM_32
> > > WRITE_CP32(n->arch.joscr, JOSCR);
> > > WRITE_CP32(n->arch.jmcr, JMCR);
> > > #endif
> > > isb();
> > > /* VFP - call vfp_restore_state after writing on CPTR_EL2 + isb */
> > > vfp_restore_state(n);
> > > Maybe I misunderstood your preference, do you want me to put the write to 
> > > CPTR_EL2
> > > right before the isb() that precedes vfp_restore_state?
> > Yes please. Unless there is a reason to keep it "far away". The comments look 
> > good to me.
> Ok, a question regarding README.LinuxPrimitives, is it some file taken from an 
> automated tool?
I am not aware of any automated tools using it. All the re-syncs I have 
seen recently were manual.
> Because I see there is some kind of structure, how can I know if my syntax is 
> correct?
There are some commands to help syncing when the file is very similar to 
Linux. In your case, I would follow what we did for the atomics as only 
some functions are the same.
Cheers,

--
Julien Grall