Re: [PATCH v1] xen/arm: arm32: Add support to identify the Cortex-R52 processor

[Ayan Kumar Halder <ayankuma@xxxxxxx>]

On 20/06/2023 21:43, Julien Grall wrote:
> Hi Ayan,
Hi Julien,
> On 20/06/2023 19:28, Ayan Kumar Halder wrote:
> > On 20/06/2023 17:41, Julien Grall wrote:
> > > Hi,
> > Hi Julien,
> > > On 20/06/2023 16:17, Ayan Kumar Halder wrote:
> > > > Add a special configuration (CONFIG_AARCH32_V8R) to setup the 
> > > > Cortex-R52
> > > > specifics.
> > > >
> > > > Cortex-R52 is an Arm-V8R AArch32 processor.
> > > >
> > > > Refer ARM DDI 0487I.a ID081822, G8-9647, G8.2.112 MIDR,
> > > > bits[31:24] = 0x41 , Arm Ltd
> > > > bits[23:20] = Implementation defined
> > > > bits[19:16] = 0xf , Arch features are individually identified
> > > > bits[15:4] = Implementation defined
> > > > bits[3:0] = Implementation defined
> > > >
> > > > Thus, the processor id is 0x410f0000 and the processor id mask is
> > > > 0xff0f0000
> > > >
> > > > Also, there is no special initialization required for R52.
> > > Are you saying that Xen upstream + this patch will boot on Cortex-R52?
> > This patch will help for earlyboot of Xen. With this patch, 
> > cpu_init() will work on Cortex-R52.
> > There will be changes required for the MPU configuration, but that 
> > will be sent after Penny's patch serie "[PATCH v2 00/41] xen/arm: Add 
> > Armv8-R64 MPU support to Xen - Part#1" is upstreamed.
> > My aim is to extract the non-dependent changes and send them for review.
> I can review the patch. But I am not willing to merge it as it gives 
> the false impression that Xen would boot on Cortex-R52.
> In fact, I think this patch should only be merged once we have all the 
> MPU merged.
> IMHO, patches are independent are rework (e.g. code split...) that 
> would help the MPU.
Yes, that's exactly what I intend to do. I will send out the patches 
(similar to this) which will not be impacted by the MPU changes.
So, that both as an author/reviewer, it helps to restrict MPU serie to 
only MPU specific changes.
Can you suggest some change to the commit message, so that we can commit 
it (without giving any false impression that Xen boots on Cortex-R52) ?
May be adding this line to the commit message helps ?

"However, Xen does not fully boot on Cortex-R52 as it requires MPU 
support which is under review.
Once Xen is supported on Cortex-R52, SUPPORT.md will be amended to 
reflect it."
- Ayan

> Cheers,