Re: [PATCH for-4.19] ppc/riscv: fix arch_acquire_resource_check()

[Oleksii <oleksii.kurochko@xxxxxxxxx>]

On Tue, 2024-04-30 at 17:34 +0200, Roger Pau Monne wrote:
> None of the implementations support set_foreign_p2m_entry() yet,
> neither they
> have a p2m walk in domain_relinquish_resources() in order to remove
> the foreign
> mappings from the p2m and thus drop the extra refcounts.
> 
> Adjust the arch helpers to return false and introduce a comment that
> clearly
> states it is not only taking extra refcounts that's needed, but also
> dropping
> them on domain teardown.
I am okay with such adjustment for now as it is more safe and nothing
will be missed during implementation of p2m, but I am curious how then
Arm handles that, their implementation is also just returns true. ( I
planned to have p2m implementation similar to Arm )

Anyway, based on that it safer for RISC-V:
Reviewed-by: Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>
> 
> Fixes: 4988704e00d8 ('xen/riscv: introduce p2m.h')
> Fixes: 4a2f68f90930 ('xen/ppc: Define minimal stub headers required
> for full build')
> Signed-off-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>
> ---
>  xen/arch/ppc/include/asm/p2m.h   | 7 ++++---
>  xen/arch/riscv/include/asm/p2m.h | 7 ++++---
>  2 files changed, 8 insertions(+), 6 deletions(-)
> 
> diff --git a/xen/arch/ppc/include/asm/p2m.h
> b/xen/arch/ppc/include/asm/p2m.h
> index 25ba05466853..f144ef8e1a54 100644
> --- a/xen/arch/ppc/include/asm/p2m.h
> +++ b/xen/arch/ppc/include/asm/p2m.h
> @@ -81,10 +81,11 @@ static inline mfn_t gfn_to_mfn(struct domain *d,
> gfn_t gfn)
>  static inline bool arch_acquire_resource_check(struct domain *d)
>  {
>      /*
> -     * The reference counting of foreign entries in
> set_foreign_p2m_entry()
> -     * is supported on PPC.
> +     * Requires refcounting the foreign mappings and walking the p2m
> on
> +     * teardown in order to remove foreign pages from the p2m and
> drop the
> +     * extra reference counts.
>       */
> -    return true;
> +    return false;
>  }
>  
>  static inline void p2m_altp2m_check(struct vcpu *v, uint16_t idx)
> diff --git a/xen/arch/riscv/include/asm/p2m.h
> b/xen/arch/riscv/include/asm/p2m.h
> index 87b13f897926..387f372b5d26 100644
> --- a/xen/arch/riscv/include/asm/p2m.h
> +++ b/xen/arch/riscv/include/asm/p2m.h
> @@ -79,10 +79,11 @@ static inline mfn_t gfn_to_mfn(struct domain *d,
> gfn_t gfn)
>  static inline bool arch_acquire_resource_check(struct domain *d)
>  {
>      /*
> -     * The reference counting of foreign entries in
> set_foreign_p2m_entry()
> -     * is supported on RISCV.
> +     * Requires refcounting the foreign mappings and walking the p2m
> on
> +     * teardown in order to remove foreign pages from the p2m and
> drop the
> +     * extra reference counts.
>       */
> -    return true;
> +    return false;
>  }
>  
>  static inline void p2m_altp2m_check(struct vcpu *v, uint16_t idx)