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

[Julien Grall <julien@xxxxxxx>]

Hi Ayan,

On 22/06/2023 09:59, Ayan Kumar Halder wrote:
> 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."
While writing an answer for this patch, I was actually wondering whether 
the CPU allowlist still make sense for the 32-bit ARMv8-R?
From Armv7-A, we needed it because some CPUs need specific quirk when 
booting. But it would be best if we can get rid of it for 32-bit ARMv8-R.
Looking at your patch, your merely providing stubs. Do you have any plan 
to fill them up?
Cheers,

--
Julien Grall