[PATCH v3 0/4] P2M improvements for Arm

Henry Wang posted 4 patches 1 year ago
Test gitlab-ci failed
Patches applied successfully (tree, apply log)
git fetch https://gitlab.com/xen-project/patchew/xen tags/patchew/20230328071334.2098429-1-Henry.Wang@arm.com
xen/arch/arm/domain.c               | 10 ++++-
xen/arch/arm/include/asm/new_vgic.h | 10 +++--
xen/arch/arm/include/asm/p2m.h      | 15 +++----
xen/arch/arm/include/asm/vgic.h     |  2 +
xen/arch/arm/p2m.c                  | 64 +++++++++++------------------
xen/arch/arm/traps.c                | 19 +++++++--
xen/arch/arm/vgic-v2.c              | 25 ++++-------
xen/arch/arm/vgic/vgic-init.c       |  4 +-
xen/arch/arm/vgic/vgic-v2.c         | 41 +++++++-----------
9 files changed, 85 insertions(+), 105 deletions(-)
[PATCH v3 0/4] P2M improvements for Arm
Posted by Henry Wang 1 year ago
There are some clean-up/improvement work that can be done in the
Arm P2M code triggered by [1] and [2]. These were found at the 4.17
code freeze period so the issues were not fixed at that time.
Therefore do the follow-ups here.

Patch#1 addresses one comment in [1]. It was sent earlier and reviewed
once. Pick the updated version, i.e. "[PATCH v2] xen/arm: Reduce
redundant clear root pages when teardown p2m", to this series.

Patch#2 is a new patch based on v1 comments, this is a pre-requisite
patch for patch#3 where the deferring of GICv2 CPU interface mapping
should also be applied for new vGIC.

Patch#3 and #4 addresses the comment in [2] following the discussion
between two possible options.

[1] https://lore.kernel.org/xen-devel/a947e0b4-8f76-cea6-893f-abf30ff95e0d@xen.org/
[2] https://lore.kernel.org/xen-devel/e6643bfc-5bdf-f685-1b68-b28d341071c1@xen.org/

v2 -> v3:
1. Add Julien's acked-by tag for patch #2.
2. Reword the reason why hwdom extra mappings are not touched by this
   patch in the commit message of patch #3.
3. Rework the address check in stage-2 data abort trap so that larger
   CPU interface size can work fine.
4. Correct a typo in original in-code comment, slightly modify
   the wording to avoid the presence of preemptive/non-preemptive
   p2m_teardown() call assumption.
5. Drop the (now) unnecessary second parameter of p2m_teardown().

v1 -> v2:
1. Move in-code comment for p2m_force_tlb_flush_sync() on top of
   p2m_clear_root_pages().
2. Add a new patch as patch #2.
3. Correct style in in-code comment in patch #3.
4. Avoid open-coding gfn_eq() and gaddr_to_gfn(d->arch.vgic.cbase).
5. Apply same changes for the new vGICv2 implementation, update the
   commit message accordingly.
6. Add in-code comment in old GICv2's vgic_v2_domain_init() and
   new GICv2's vgic_v2_map_resources() to mention the mapping of the
   virtual CPU interface is deferred until first access.
7. Add reviewed-by and acked-by tags accordingly.

Henry Wang (4):
  xen/arm: Reduce redundant clear root pages when teardown p2m
  xen/arm: Rename vgic_cpu_base and vgic_dist_base for new vGIC
  xen/arm: Defer GICv2 CPU interface mapping until the first access
  xen/arm: Clean-up in p2m_init() and p2m_final_teardown()

 xen/arch/arm/domain.c               | 10 ++++-
 xen/arch/arm/include/asm/new_vgic.h | 10 +++--
 xen/arch/arm/include/asm/p2m.h      | 15 +++----
 xen/arch/arm/include/asm/vgic.h     |  2 +
 xen/arch/arm/p2m.c                  | 64 +++++++++++------------------
 xen/arch/arm/traps.c                | 19 +++++++--
 xen/arch/arm/vgic-v2.c              | 25 ++++-------
 xen/arch/arm/vgic/vgic-init.c       |  4 +-
 xen/arch/arm/vgic/vgic-v2.c         | 41 +++++++-----------
 9 files changed, 85 insertions(+), 105 deletions(-)

-- 
2.25.1
Re: [PATCH v3 0/4] P2M improvements for Arm
Posted by Julien Grall 11 months, 2 weeks ago
Hi Henry,

On 28/03/2023 08:13, Henry Wang wrote:
> There are some clean-up/improvement work that can be done in the
> Arm P2M code triggered by [1] and [2]. These were found at the 4.17
> code freeze period so the issues were not fixed at that time.
> Therefore do the follow-ups here.
> 
> Patch#1 addresses one comment in [1]. It was sent earlier and reviewed
> once. Pick the updated version, i.e. "[PATCH v2] xen/arm: Reduce
> redundant clear root pages when teardown p2m", to this series.
> 
> Patch#2 is a new patch based on v1 comments, this is a pre-requisite
> patch for patch#3 where the deferring of GICv2 CPU interface mapping
> should also be applied for new vGIC.
> 
> Patch#3 and #4 addresses the comment in [2] following the discussion
> between two possible options.
> 
> [1] https://lore.kernel.org/xen-devel/a947e0b4-8f76-cea6-893f-abf30ff95e0d@xen.org/
> [2] https://lore.kernel.org/xen-devel/e6643bfc-5bdf-f685-1b68-b28d341071c1@xen.org/
> 
> v2 -> v3:
> 1. Add Julien's acked-by tag for patch #2.
> 2. Reword the reason why hwdom extra mappings are not touched by this
>     patch in the commit message of patch #3.
> 3. Rework the address check in stage-2 data abort trap so that larger
>     CPU interface size can work fine.
> 4. Correct a typo in original in-code comment, slightly modify
>     the wording to avoid the presence of preemptive/non-preemptive
>     p2m_teardown() call assumption.
> 5. Drop the (now) unnecessary second parameter of p2m_teardown().
> 
> v1 -> v2:
> 1. Move in-code comment for p2m_force_tlb_flush_sync() on top of
>     p2m_clear_root_pages().
> 2. Add a new patch as patch #2.
> 3. Correct style in in-code comment in patch #3.
> 4. Avoid open-coding gfn_eq() and gaddr_to_gfn(d->arch.vgic.cbase).
> 5. Apply same changes for the new vGICv2 implementation, update the
>     commit message accordingly.
> 6. Add in-code comment in old GICv2's vgic_v2_domain_init() and
>     new GICv2's vgic_v2_map_resources() to mention the mapping of the
>     virtual CPU interface is deferred until first access.
> 7. Add reviewed-by and acked-by tags accordingly.
> 
> Henry Wang (4):
>    xen/arm: Reduce redundant clear root pages when teardown p2m
>    xen/arm: Rename vgic_cpu_base and vgic_dist_base for new vGIC
>    xen/arm: Defer GICv2 CPU interface mapping until the first access
>    xen/arm: Clean-up in p2m_init() and p2m_final_teardown()

I have committed the series.

Cheers,

-- 
Julien Grall
RE: [PATCH v3 0/4] P2M improvements for Arm
Posted by Henry Wang 11 months, 2 weeks ago
Hi Julien,

> -----Original Message-----
> From: Julien Grall <julien@xen.org>
> Subject: Re: [PATCH v3 0/4] P2M improvements for Arm
> 
> Hi Henry,
> 
> > Henry Wang (4):
> >    xen/arm: Reduce redundant clear root pages when teardown p2m
> >    xen/arm: Rename vgic_cpu_base and vgic_dist_base for new vGIC
> >    xen/arm: Defer GICv2 CPU interface mapping until the first access
> >    xen/arm: Clean-up in p2m_init() and p2m_final_teardown()
> 
> I have committed the series.

Many thanks for that! Also thanks for your review for the series.

While you are at this, I am wondering would you mind also taking a look
at [1] since I believe I have fixed your comments which correctly pointed
out about the format of "printk info" in v3 of that series.

If you have more comments, please don't hesitate to raise them and I
will take care of them tomorrow :))

Thanks!

[1] https://lore.kernel.org/xen-devel/20230201021513.336837-1-Henry.Wang@arm.com/

Kind regards,
Henry

> 
> Cheers,
> 
> --
> Julien Grall
Re: [PATCH v3 0/4] P2M improvements for Arm
Posted by Julien Grall 11 months, 2 weeks ago

On 16/04/2023 16:18, Henry Wang wrote:
> Hi Julien,

Hi Henry,

>> -----Original Message-----
>> From: Julien Grall <julien@xen.org>
>> Subject: Re: [PATCH v3 0/4] P2M improvements for Arm
>>
>> Hi Henry,
>>
>>> Henry Wang (4):
>>>     xen/arm: Reduce redundant clear root pages when teardown p2m
>>>     xen/arm: Rename vgic_cpu_base and vgic_dist_base for new vGIC
>>>     xen/arm: Defer GICv2 CPU interface mapping until the first access
>>>     xen/arm: Clean-up in p2m_init() and p2m_final_teardown()
>>
>> I have committed the series.
> 
> Many thanks for that! Also thanks for your review for the series.
> 
> While you are at this, I am wondering would you mind also taking a look
> at [1] since I believe I have fixed your comments which correctly pointed
> out about the format of "printk info" in v3 of that series.

Sorry this fell through the cracks. I will have a look at it now.

Cheers,

-- 
Julien Grall