Re: [XEN v1 3/4] xen/arm32: Split and move MMU-specific head.S to mmu/head.S

[Ayan Kumar Halder <ayankuma@xxxxxxx>]

On 16/10/2023 19:45, Julien Grall wrote:
> Hi,
Hi Julien,
> On 11/09/2023 14:59, Ayan Kumar Halder wrote:
> > This is based on
> > "[PATCH v6 03/13] xen/arm64: Split and move MMU-specific head.S to 
> > mmu/head.S"
> > https://www.mail-archive.com/xen-devel@xxxxxxxxxxxxxxxxxxxx/msg151920.html 
> Same remark as patch #1. Please provide a proper commit message.
>
> [...]
>
> > diff --git a/xen/arch/arm/include/asm/arm32/macros.h 
> > b/xen/arch/arm/include/asm/arm32/macros.h
> > index a4e20aa520..64fbe4876a 100644
> > --- a/xen/arch/arm/include/asm/arm32/macros.h
> > +++ b/xen/arch/arm/include/asm/arm32/macros.h
> > @@ -1,8 +1,70 @@
> >   #ifndef __ASM_ARM_ARM32_MACROS_H
> >   #define __ASM_ARM_ARM32_MACROS_H
> >   +/* Offset between the early boot xen mapping and the runtime xen 
> > mapping */
> > +#define XEN_TEMPORARY_OFFSET      (TEMPORARY_XEN_VIRT_START - 
> > XEN_VIRT_START)
> > +
> >       .macro ret
> >           mov     pc, lr
> >       .endm
> >   +/*
> > + * Move an immediate constant into a 32-bit register using movw/movt
> > + * instructions.
> > + */
> > +.macro mov_w reg, word
> > +        movw  \reg, #:lower16:\word
> > +        movt  \reg, #:upper16:\word
> > +.endm
> > +
> > +/*
> > + * Pseudo-op for PC relative adr <reg>, <symbol> where <symbol> is
> > + * within the range +/- 4GB of the PC.
> > + *
> > + * @dst: destination register
> > + * @sym: name of the symbol
> > + */
> > +.macro adr_l, dst, sym
> > +        mov_w \dst, \sym - .Lpc\@
> > +        .set  .Lpc\@, .+ 8          /* PC bias */
> > +        add   \dst, \dst, pc
> > +.endm
> > +
> > +.macro load_paddr rb, sym
> > +        mov_w \rb, \sym
> > +        add   \rb, \rb, r10
> > +.endm
> I see that we are still using load_paddr in arm64/head.S. But I don't 
> think it makes entirely sense as r10 (x20) would always be 0 for the MPU.
> In fact all the use in arm{32, 64}/head.S after this patch could be 
> easily replaced with adr_l because they are called when the MMU is off 
> so the return address will be a physical address.
> So can you add a prequisite patch for arm32 to replace the remaining 
> some of the load_paddr? With that, load_paddr can be moved in mmu/head.S.
Sure, I am doing this. I will send you a patch in next few days.
> I will take care of the arm64 part.
I see you have already sent the patch. :)

- Ayan

> Cheers,