As a follow up to commit cb1ddafdc573 ("xen/arm/static-shmem: Static-shmem
should be direct-mapped for direct-mapped domains") add a check to
request that both host and guest physical address must be supplied for
direct mapped domains. Otherwise return an error to prevent unwanted
behavior.

Signed-off-by: Michal Orzel <michal.orzel@amd.com>
---
Reasoning for 4.19:
this is hardening the code to prevent a feature misuse and unwanted behavior.
---
 xen/arch/arm/static-shmem.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/xen/arch/arm/static-shmem.c b/xen/arch/arm/static-shmem.c
index cd48d2896b7e..aa80756c3ca5 100644
--- a/xen/arch/arm/static-shmem.c
+++ b/xen/arch/arm/static-shmem.c
@@ -378,6 +378,13 @@ int __init process_shm(struct domain *d, struct kernel_info *kinfo,
             const struct membank *alloc_bank =
                 find_shm_bank_by_id(get_shmem_heap_banks(), shm_id);
 
+            if ( is_domain_direct_mapped(d) )
+            {
+                printk("%pd: host and guest physical address must be supplied for direct-mapped domains\n",
+                       d);
+                return -EINVAL;
+            }
+
             /* guest phys address is right at the beginning */
             gbase = dt_read_paddr(cells, addr_cells);
 
-- 
2.25.1

Hi Michal,

On 21/06/2024 10:22, Michal Orzel wrote:
> As a follow up to commit cb1ddafdc573 ("xen/arm/static-shmem: Static-shmem
> should be direct-mapped for direct-mapped domains") add a check to
> request that both host and guest physical address must be supplied for
> direct mapped domains. Otherwise return an error to prevent unwanted
> behavior.
> 
> Signed-off-by: Michal Orzel <michal.orzel@amd.com>

I would argue that it should be have a fixes tag:

Fixes: 988f1c7e1f40 ("xen/arm: static-shmem: fix "gbase/pbase used 
uninitialized" build failure")

Mainly because while you fixed the build, it was missing the check in 
the "else" part.

I am happy to update it on commit if you are ok with the change.

Reviewed-by: Julien Grall <jgrall@amazon.com>

Cheers,

-- 
Julien Grall

Hi Julien,

On 24/06/2024 12:22, Julien Grall wrote:
> 
> 
> Hi Michal,
> 
> On 21/06/2024 10:22, Michal Orzel wrote:
>> As a follow up to commit cb1ddafdc573 ("xen/arm/static-shmem: Static-shmem
>> should be direct-mapped for direct-mapped domains") add a check to
>> request that both host and guest physical address must be supplied for
>> direct mapped domains. Otherwise return an error to prevent unwanted
>> behavior.
>>
>> Signed-off-by: Michal Orzel <michal.orzel@amd.com>
> 
> I would argue that it should be have a fixes tag:
> 
> Fixes: 988f1c7e1f40 ("xen/arm: static-shmem: fix "gbase/pbase used
> uninitialized" build failure")
> 
> Mainly because while you fixed the build, it was missing the check in
> the "else" part.
> 
> I am happy to update it on commit if you are ok with the change.Yes, I'm fine with it.

> 
> Reviewed-by: Julien Grall <jgrall@amazon.com>
> 
> Cheers,
> 
> --
> Julien Grall

~Michal

On 24/06/2024 11:43, Michal Orzel wrote:
> Hi Julien,
> 
> On 24/06/2024 12:22, Julien Grall wrote:
>>
>>
>> Hi Michal,
>>
>> On 21/06/2024 10:22, Michal Orzel wrote:
>>> As a follow up to commit cb1ddafdc573 ("xen/arm/static-shmem: Static-shmem
>>> should be direct-mapped for direct-mapped domains") add a check to
>>> request that both host and guest physical address must be supplied for
>>> direct mapped domains. Otherwise return an error to prevent unwanted
>>> behavior.
>>>
>>> Signed-off-by: Michal Orzel <michal.orzel@amd.com>
>>
>> I would argue that it should be have a fixes tag:
>>
>> Fixes: 988f1c7e1f40 ("xen/arm: static-shmem: fix "gbase/pbase used
>> uninitialized" build failure")
>>
>> Mainly because while you fixed the build, it was missing the check in
>> the "else" part.
>>
>> I am happy to update it on commit if you are ok with the change.
> Yes, I'm fine with it.

Committed.

Cheers,

-- 
Julien Grall

On Fri, 2024-06-21 at 11:22 +0200, Michal Orzel wrote:
> As a follow up to commit cb1ddafdc573 ("xen/arm/static-shmem: Static-
> shmem
> should be direct-mapped for direct-mapped domains") add a check to
> request that both host and guest physical address must be supplied
> for
> direct mapped domains. Otherwise return an error to prevent unwanted
> behavior.
> 
> Signed-off-by: Michal Orzel <michal.orzel@amd.com>
> ---
> Reasoning for 4.19:
> this is hardening the code to prevent a feature misuse and unwanted
> behavior.
> ---
Release-Acked-By: Oleksii Kurochko <oleksii.kurochko@gmail.com>

~ Oleksii
>  xen/arch/arm/static-shmem.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/xen/arch/arm/static-shmem.c b/xen/arch/arm/static-
> shmem.c
> index cd48d2896b7e..aa80756c3ca5 100644
> --- a/xen/arch/arm/static-shmem.c
> +++ b/xen/arch/arm/static-shmem.c
> @@ -378,6 +378,13 @@ int __init process_shm(struct domain *d, struct
> kernel_info *kinfo,
>              const struct membank *alloc_bank =
>                  find_shm_bank_by_id(get_shmem_heap_banks(), shm_id);
>  
> +            if ( is_domain_direct_mapped(d) )
> +            {
> +                printk("%pd: host and guest physical address must be
> supplied for direct-mapped domains\n",
> +                       d);
> +                return -EINVAL;
> +            }
> +
>              /* guest phys address is right at the beginning */
>              gbase = dt_read_paddr(cells, addr_cells);
>