Re: [PATCH v3 1/4] xen/arm/static-shmem: Static-shmem should be direct-mapped for direct-mapped domains

[Michal Orzel <michal.orzel@xxxxxxx>]

Hi Henry,

+CC: Luca

On 17/05/2024 05:21, Henry Wang wrote:
> 
> 
> Currently, users are allowed to map static shared memory in a
> non-direct-mapped way for direct-mapped domains. This can lead to
> clashing of guest memory spaces. Also, the current extended region
> finding logic only removes the host physical addresses of the
> static shared memory areas for direct-mapped domains, which may be
> inconsistent with the guest memory map if users map the static
> shared memory in a non-direct-mapped way. This will lead to incorrect
> extended region calculation results.
> 
> To make things easier, add restriction that static shared memory
> should also be direct-mapped for direct-mapped domains. Check the
> host physical address to be matched with guest physical address when
> parsing the device tree. Document this restriction in the doc.
I'm ok with this restriction.

@Luca, do you have any use case preventing us from making this restriction?

This patch clashes with Luca series so depending on which goes first,
Acked-by: Michal Orzel <michal.orzel@xxxxxxx>

> 
> Signed-off-by: Henry Wang <xin.wang2@xxxxxxx>
> ---
> v3:
> - New patch.
> ---
>  docs/misc/arm/device-tree/booting.txt | 3 +++
>  xen/arch/arm/static-shmem.c           | 6 ++++++
>  2 files changed, 9 insertions(+)
> 
> diff --git a/docs/misc/arm/device-tree/booting.txt 
> b/docs/misc/arm/device-tree/booting.txt
> index bbd955e9c2..c994e48391 100644
> --- a/docs/misc/arm/device-tree/booting.txt
> +++ b/docs/misc/arm/device-tree/booting.txt
> @@ -591,6 +591,9 @@ communication.
>      shared memory region in host physical address space, a size, and a guest
>      physical address, as the target address of the mapping.
>      e.g. xen,shared-mem = < [host physical address] [guest address] [size] >
> +    Note that if a domain is direct-mapped, i.e. the Dom0 and the Dom0less
> +    DomUs with `direct-map` device tree property, the static shared memory
> +    should also be direct-mapped (host physical address == guest address).
> 
>      It shall also meet the following criteria:
>      1) If the SHM ID matches with an existing region, the address range of 
> the
> diff --git a/xen/arch/arm/static-shmem.c b/xen/arch/arm/static-shmem.c
> index 78881dd1d3..b26fb69874 100644
> --- a/xen/arch/arm/static-shmem.c
> +++ b/xen/arch/arm/static-shmem.c
> @@ -235,6 +235,12 @@ int __init process_shm(struct domain *d, struct 
> kernel_info *kinfo,
>                     d, psize);
>              return -EINVAL;
>          }
> +        if ( is_domain_direct_mapped(d) && (pbase != gbase) )
> +        {
> +            printk("%pd: physical address 0x%"PRIpaddr" and guest address 
> 0x%"PRIpaddr" are not 1:1 direct-mapped.\n",
NIT: 1:1 and direct-mapped means the same so no need to place them next to each 
other

> +                   d, pbase, gbase);
> +            return -EINVAL;
> +        }
> 
>          for ( i = 0; i < PFN_DOWN(psize); i++ )
>              if ( !mfn_valid(mfn_add(maddr_to_mfn(pbase), i)) )
> --
> 2.34.1
> 
> 

~Michal