On Mon, 7 Nov 2022, Wei Chen wrote:
> Hi Julien,
> 
> > -----Original Message-----
> > From: Julien Grall <julien@xen.org>
> > Sent: 2022年11月7日 18:16
> > To: Wei Chen <Wei.Chen@arm.com>; xen-devel@lists.xenproject.org
> > Cc: nd <nd@arm.com>; Stefano Stabellini <sstabellini@kernel.org>; Bertrand
> > Marquis <Bertrand.Marquis@arm.com>; Volodymyr Babchuk
> > <Volodymyr_Babchuk@epam.com>
> > Subject: Re: [PATCH v6 00/11] xen/arm: Add Armv8-R64 MPU support to Xen -
> > Part#1
> > 
> > 
> > 
> > On 07/11/2022 09:52, Wei Chen wrote:
> > > Hi Julien,
> > 
> > Hi,
> > 
> > >
> > >>>    - Supports only a single Security state - Secure.
> > >>>    - MPU in EL1 & EL2 is configurable, MMU in EL1 is configurable.
> > >>>
> > >>> These patch series are implementing the Armv8-R64 MPU support
> > >>> for Xen, which are based on the discussion of
> > >>> "Proposal for Porting Xen to Armv8-R64 - DraftC" [1].
> > >>>
> > >>> We will implement the Armv8-R64 and MPU support in three stages:
> > >>> 1. Boot Xen itself to idle thread, do not create any guests on it.
> > >>
> > >> I read this as I can build Xen and see it boots (not creating domain).
> > >> However... HAS_MPU is not defined and I was expecting mm.c to be
> > >> modified to cater the MPU support. So I am a bit ensure what the series
> > >> is actually doing.
> > >>
> > >
> > > These 11 patches are part#1 of stage#1, the full stage#1 has about 30
> > > patches. We have some concerns if we send so many patches at once, the
> > > review pressure of maintainers may be very high, so we only choose about
> > > 10 to send as part of it. But this also means that we can't do a
> > relatively
> > > complete thing in this part#1 series.
> > >
> > > We want to hear some suggestions from you to make so many patches can be
> > > Reviewed efficiently. Can we send the patches by stages, even the
> > stage#1
> > > will have about 30 patches?
> > 
> > 30 patches in a go is no too bad. I would personally prefer that because
> > at least I have better idea of the shape of the code after stage#1 and
> > also possibly test it (I need to check if I have access for the ARMv8-R
> > model).
> > 
> 
> I also prefer to this way. After we have addressed the comments in
> this series, we will send the full stage#1 patches together in v2.


One suggestion to make things easier to review and to commit is to
organize the series in a way so that the first 10 patches can still be
committed first independently, even if all 30 patches are sent together.

Or alternatively only send 10 patches but also add a link to a
github/gitlab tree with all the 30+ patches so that maintainers can have
a look how the whole work fit together.

I think we are all on the same page -- I just wanted to highlight that
we don't have to finish the review of all 30 patches before we can start
committing some of the initial patches in the series.

Cheers,

Stefano

Hi Stefano, Julien,

> -----Original Message-----
> From: Stefano Stabellini <sstabellini@kernel.org>
> Sent: 2022年11月11日 6:26
> To: Wei Chen <Wei.Chen@arm.com>
> Cc: Julien Grall <julien@xen.org>; xen-devel@lists.xenproject.org; nd
> <nd@arm.com>; Stefano Stabellini <sstabellini@kernel.org>; Bertrand
> Marquis <Bertrand.Marquis@arm.com>; Volodymyr Babchuk
> <Volodymyr_Babchuk@epam.com>
> Subject: RE: [PATCH v6 00/11] xen/arm: Add Armv8-R64 MPU support to Xen -
> Part#1
> 
> On Mon, 7 Nov 2022, Wei Chen wrote:
> > Hi Julien,
> >
> > > -----Original Message-----
> > > From: Julien Grall <julien@xen.org>
> > > Sent: 2022年11月7日 18:16
> > > To: Wei Chen <Wei.Chen@arm.com>; xen-devel@lists.xenproject.org
> > > Cc: nd <nd@arm.com>; Stefano Stabellini <sstabellini@kernel.org>;
> Bertrand
> > > Marquis <Bertrand.Marquis@arm.com>; Volodymyr Babchuk
> > > <Volodymyr_Babchuk@epam.com>
> > > Subject: Re: [PATCH v6 00/11] xen/arm: Add Armv8-R64 MPU support to
> Xen -
> > > Part#1
> > >
> > >
> > >
> > > On 07/11/2022 09:52, Wei Chen wrote:
> > > > Hi Julien,
> > >
> > > Hi,
> > >
> > > >
> > > >>>    - Supports only a single Security state - Secure.
> > > >>>    - MPU in EL1 & EL2 is configurable, MMU in EL1 is configurable.
> > > >>>
> > > >>> These patch series are implementing the Armv8-R64 MPU support
> > > >>> for Xen, which are based on the discussion of
> > > >>> "Proposal for Porting Xen to Armv8-R64 - DraftC" [1].
> > > >>>
> > > >>> We will implement the Armv8-R64 and MPU support in three stages:
> > > >>> 1. Boot Xen itself to idle thread, do not create any guests on it.
> > > >>
> > > >> I read this as I can build Xen and see it boots (not creating
> domain).
> > > >> However... HAS_MPU is not defined and I was expecting mm.c to be
> > > >> modified to cater the MPU support. So I am a bit ensure what the
> series
> > > >> is actually doing.
> > > >>
> > > >
> > > > These 11 patches are part#1 of stage#1, the full stage#1 has about
> 30
> > > > patches. We have some concerns if we send so many patches at once,
> the
> > > > review pressure of maintainers may be very high, so we only choose
> about
> > > > 10 to send as part of it. But this also means that we can't do a
> > > relatively
> > > > complete thing in this part#1 series.
> > > >
> > > > We want to hear some suggestions from you to make so many patches
> can be
> > > > Reviewed efficiently. Can we send the patches by stages, even the
> > > stage#1
> > > > will have about 30 patches?
> > >
> > > 30 patches in a go is no too bad. I would personally prefer that
> because
> > > at least I have better idea of the shape of the code after stage#1 and
> > > also possibly test it (I need to check if I have access for the ARMv8-
> R
> > > model).
> > >
> >
> > I also prefer to this way. After we have addressed the comments in
> > this series, we will send the full stage#1 patches together in v2.
> 
> 
> One suggestion to make things easier to review and to commit is to
> organize the series in a way so that the first 10 patches can still be
> committed first independently, even if all 30 patches are sent together.
> 

I think this is foreseeable, and we have done in this way internally.
Every patch can be built and will not broken other architectures.

> Or alternatively only send 10 patches but also add a link to a
> github/gitlab tree with all the 30+ patches so that maintainers can have
> a look how the whole work fit together.
> 

In this series we have linked the gitlab branch with the full patches.

> I think we are all on the same page -- I just wanted to highlight that
> we don't have to finish the review of all 30 patches before we can start
> committing some of the initial patches in the series.
> 

Yes, I agree with it. And above solutions are ok for us. There will not
be much difference in efforts for these two ways for us, so if you guys
think which method is the most efficient, I will follow it.

Cheers,
Wei Chen

> Cheers,
> 
> Stefano