RE: [PATCH v2 08/40] xen/arm: use PA == VA for EARLY_UART_VIRTUAL_ADDRESS on Armv-8R

[Wei Chen <Wei.Chen@xxxxxxx>]

Hi Julien,

> -----Original Message-----
> From: Julien Grall <julien@xxxxxxx>
> Sent: 2023年1月18日 7:49
> To: Penny Zheng <Penny.Zheng@xxxxxxx>; xen-devel@xxxxxxxxxxxxxxxxxxxx
> Cc: Wei Chen <Wei.Chen@xxxxxxx>; Stefano Stabellini
> <sstabellini@xxxxxxxxxx>; Bertrand Marquis <Bertrand.Marquis@xxxxxxx>;
> Volodymyr Babchuk <Volodymyr_Babchuk@xxxxxxxx>
> Subject: Re: [PATCH v2 08/40] xen/arm: use PA == VA for
> EARLY_UART_VIRTUAL_ADDRESS on Armv-8R
> 
> Hi Penny,
> 
> On 13/01/2023 05:28, Penny Zheng wrote:
> > From: Wei Chen <wei.chen@xxxxxxx>
> >
> > There is no VMSA support on Armv8-R AArch64, so we can not map early
> > UART to FIXMAP_CONSOLE. Instead, we use PA == VA to define
> > EARLY_UART_VIRTUAL_ADDRESS on Armv8-R AArch64.
> >
> > Signed-off-by: Wei Chen <wei.chen@xxxxxxx>
> 
> Your signed-off-by is missing.
> 
> > ---
> > 1. New patch
> > ---
> >   xen/arch/arm/include/asm/early_printk.h | 12 ++++++++++++
> >   1 file changed, 12 insertions(+)
> >
> > diff --git a/xen/arch/arm/include/asm/early_printk.h
> b/xen/arch/arm/include/asm/early_printk.h
> > index c5149b2976..44a230853f 100644
> > --- a/xen/arch/arm/include/asm/early_printk.h
> > +++ b/xen/arch/arm/include/asm/early_printk.h
> > @@ -15,10 +15,22 @@
> >
> >   #ifdef CONFIG_EARLY_PRINTK
> >
> > +#ifdef CONFIG_ARM_V8R
> 
> Shouldn't this be CONFIG_HAS_MPU?
> 

We had considered that there may be an implementation of Arm8R without
an MPU, so we used CONFIG_ARM_V8R here. But you're right, we have not
support non-MPU scenario in this series, so use CONFIG_HAS_MPU here
would be better to indicate this is a feature based code section.
We will change it to CONFIG_HAS_MPU in next version.

> > +
> > +/*
> > + * For Armv-8r, there is not VMSA support in EL2, so we use VA == PA
> 
> s/not/no/
> 

Ok.

Cheers,
Wei Chen

> > + * for EARLY_UART_VIRTUAL_ADDRESS. > + */
> > +#define EARLY_UART_VIRTUAL_ADDRESS CONFIG_EARLY_UART_BASE_ADDRESS
> > +
> > +#else
> > +
> >   /* need to add the uart address offset in page to the fixmap address
> */
> >   #define EARLY_UART_VIRTUAL_ADDRESS \
> >       (FIXMAP_ADDR(FIXMAP_CONSOLE) + (CONFIG_EARLY_UART_BASE_ADDRESS &
> ~PAGE_MASK))
> >
> > +#endif /* CONFIG_ARM_V8R */
> > +
> >   #endif /* !CONFIG_EARLY_PRINTK */
> >
> >   #endif
> 
> Cheers,
> 
> --
> Julien Grall