[PATCH v3 2/5] xen/arm: dom0less: Add trap-unmapped-accesses

Edgar E. Iglesias posted 5 patches 5 months ago
There is a newer version of this series
[PATCH v3 2/5] xen/arm: dom0less: Add trap-unmapped-accesses
Posted by Edgar E. Iglesias 5 months ago
From: "Edgar E. Iglesias" <edgar.iglesias@amd.com>

Add the trap-unmapped-accesses per-domain fdt property.

Signed-off-by: Edgar E. Iglesias <edgar.iglesias@amd.com>
---
 docs/misc/arm/device-tree/booting.txt | 10 ++++++++++
 xen/arch/arm/dom0less-build.c         |  9 ++++++++-
 2 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/docs/misc/arm/device-tree/booting.txt b/docs/misc/arm/device-tree/booting.txt
index 59fa96a82e..9add6440de 100644
--- a/docs/misc/arm/device-tree/booting.txt
+++ b/docs/misc/arm/device-tree/booting.txt
@@ -225,6 +225,16 @@ with the following properties:
     option is provided with a non zero value, but the platform doesn't support
     SVE.
 
+- trap-unmapped-accesses
+
+    Optional. An integer that configures handling of accesses to unmapped
+    address ranges.
+    If set to 0, guest accesses will read all bits as ones, e.g 0xFFFFFFFF
+    for a 32bit access and writes will be ignored.
+    If set to 1, guest accesses will trap.
+
+    This option is only implemented for ARM where the default is 1.
+
 - xen,enhanced
 
     A string property. Possible property values are:
diff --git a/xen/arch/arm/dom0less-build.c b/xen/arch/arm/dom0less-build.c
index a4e0a33632..69324aa597 100644
--- a/xen/arch/arm/dom0less-build.c
+++ b/xen/arch/arm/dom0less-build.c
@@ -344,8 +344,15 @@ void __init arch_create_domUs(struct dt_device_node *node,
 #endif
     }
 
-    /* Trap accesses to unmapped areas. */
+    /* Trap unmapped accesses by default. */
     d_cfg->flags |= XEN_DOMCTL_CDF_trap_unmapped_accesses;
+    if ( dt_property_read_u32(node, "trap-unmapped-accesses", &val) )
+    {
+        if ( val > 1 )
+            panic("trap-unmapped-accesses: supported values are 0 or 1");
+        if ( !val )
+            d_cfg->flags &= ~XEN_DOMCTL_CDF_trap_unmapped_accesses;
+    }
 }
 
 int __init init_intc_phandle(struct kernel_info *kinfo, const char *name,
-- 
2.43.0
Re: [PATCH v3 2/5] xen/arm: dom0less: Add trap-unmapped-accesses
Posted by Stefano Stabellini 5 months ago
On Fri, 30 May 2025, Edgar E. Iglesias wrote:
> From: "Edgar E. Iglesias" <edgar.iglesias@amd.com>
> 
> Add the trap-unmapped-accesses per-domain fdt property.
> 
> Signed-off-by: Edgar E. Iglesias <edgar.iglesias@amd.com>
> ---
>  docs/misc/arm/device-tree/booting.txt | 10 ++++++++++
>  xen/arch/arm/dom0less-build.c         |  9 ++++++++-
>  2 files changed, 18 insertions(+), 1 deletion(-)
> 
> diff --git a/docs/misc/arm/device-tree/booting.txt b/docs/misc/arm/device-tree/booting.txt
> index 59fa96a82e..9add6440de 100644
> --- a/docs/misc/arm/device-tree/booting.txt
> +++ b/docs/misc/arm/device-tree/booting.txt
> @@ -225,6 +225,16 @@ with the following properties:
>      option is provided with a non zero value, but the platform doesn't support
>      SVE.
>  
> +- trap-unmapped-accesses
> +
> +    Optional. An integer that configures handling of accesses to unmapped
> +    address ranges.
> +    If set to 0, guest accesses will read all bits as ones, e.g 0xFFFFFFFF
> +    for a 32bit access and writes will be ignored.
> +    If set to 1, guest accesses will trap.
> +
> +    This option is only implemented for ARM where the default is 1.

Please expand it to: "This option is only implemented for ARM where the
default is 1 when trap-unmapped-accesses is absent."

The change could be done on commit:

Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>


>  - xen,enhanced
>  
>      A string property. Possible property values are:
> diff --git a/xen/arch/arm/dom0less-build.c b/xen/arch/arm/dom0less-build.c
> index a4e0a33632..69324aa597 100644
> --- a/xen/arch/arm/dom0less-build.c
> +++ b/xen/arch/arm/dom0less-build.c
> @@ -344,8 +344,15 @@ void __init arch_create_domUs(struct dt_device_node *node,
>  #endif
>      }
>  
> -    /* Trap accesses to unmapped areas. */
> +    /* Trap unmapped accesses by default. */
>      d_cfg->flags |= XEN_DOMCTL_CDF_trap_unmapped_accesses;
> +    if ( dt_property_read_u32(node, "trap-unmapped-accesses", &val) )
> +    {
> +        if ( val > 1 )
> +            panic("trap-unmapped-accesses: supported values are 0 or 1");
> +        if ( !val )
> +            d_cfg->flags &= ~XEN_DOMCTL_CDF_trap_unmapped_accesses;
> +    }
>  }
>  
>  int __init init_intc_phandle(struct kernel_info *kinfo, const char *name,
> -- 
> 2.43.0
>
Re: [PATCH v3 2/5] xen/arm: dom0less: Add trap-unmapped-accesses
Posted by Julien Grall 5 months ago
Hi,

On 02/06/2025 23:36, Stefano Stabellini wrote:
> On Fri, 30 May 2025, Edgar E. Iglesias wrote:
>> From: "Edgar E. Iglesias" <edgar.iglesias@amd.com>
>>
>> Add the trap-unmapped-accesses per-domain fdt property.
>>
>> Signed-off-by: Edgar E. Iglesias <edgar.iglesias@amd.com>
>> ---
>>   docs/misc/arm/device-tree/booting.txt | 10 ++++++++++
>>   xen/arch/arm/dom0less-build.c         |  9 ++++++++-
>>   2 files changed, 18 insertions(+), 1 deletion(-)
>>
>> diff --git a/docs/misc/arm/device-tree/booting.txt b/docs/misc/arm/device-tree/booting.txt
>> index 59fa96a82e..9add6440de 100644
>> --- a/docs/misc/arm/device-tree/booting.txt
>> +++ b/docs/misc/arm/device-tree/booting.txt
>> @@ -225,6 +225,16 @@ with the following properties:
>>       option is provided with a non zero value, but the platform doesn't support
>>       SVE.
>>   
>> +- trap-unmapped-accesses
>> +
>> +    Optional. An integer that configures handling of accesses to unmapped
>> +    address ranges.
>> +    If set to 0, guest accesses will read all bits as ones, e.g 0xFFFFFFFF
>> +    for a 32bit access and writes will be ignored.
>> +    If set to 1, guest accesses will trap.
>> +
>> +    This option is only implemented for ARM where the default is 1.
> 
> Please expand it to: "This option is only implemented for ARM where the
> default is 1 when trap-unmapped-accesses is absent."

I am confused. The document is part of "docs/misc/arm" and some options 
like "sve" are Arm specific. We don't mention this is Arm only because 
the documention is Arm specific.

I know that RISC-V is starting to share the bindings. So really (part 
of) the documentation should be moved to common. Until then, I think it 
is misleading to add "is only implemented for ARM".

BTW, the spelling for should be "Arm" ;).

Cheers,

-- 
Julien Grall
Re: [PATCH v3 2/5] xen/arm: dom0less: Add trap-unmapped-accesses
Posted by Stefano Stabellini 5 months ago
On Mon, 2 Jun 2025, Julien Grall wrote:
> Hi,
> 
> On 02/06/2025 23:36, Stefano Stabellini wrote:
> > On Fri, 30 May 2025, Edgar E. Iglesias wrote:
> > > From: "Edgar E. Iglesias" <edgar.iglesias@amd.com>
> > > 
> > > Add the trap-unmapped-accesses per-domain fdt property.
> > > 
> > > Signed-off-by: Edgar E. Iglesias <edgar.iglesias@amd.com>
> > > ---
> > >   docs/misc/arm/device-tree/booting.txt | 10 ++++++++++
> > >   xen/arch/arm/dom0less-build.c         |  9 ++++++++-
> > >   2 files changed, 18 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/docs/misc/arm/device-tree/booting.txt
> > > b/docs/misc/arm/device-tree/booting.txt
> > > index 59fa96a82e..9add6440de 100644
> > > --- a/docs/misc/arm/device-tree/booting.txt
> > > +++ b/docs/misc/arm/device-tree/booting.txt
> > > @@ -225,6 +225,16 @@ with the following properties:
> > >       option is provided with a non zero value, but the platform doesn't
> > > support
> > >       SVE.
> > >   +- trap-unmapped-accesses
> > > +
> > > +    Optional. An integer that configures handling of accesses to unmapped
> > > +    address ranges.
> > > +    If set to 0, guest accesses will read all bits as ones, e.g
> > > 0xFFFFFFFF
> > > +    for a 32bit access and writes will be ignored.
> > > +    If set to 1, guest accesses will trap.
> > > +
> > > +    This option is only implemented for ARM where the default is 1.
> > 
> > Please expand it to: "This option is only implemented for ARM where the
> > default is 1 when trap-unmapped-accesses is absent."
> 
> I am confused. The document is part of "docs/misc/arm" and some options like
> "sve" are Arm specific. We don't mention this is Arm only because the
> documention is Arm specific.
> 
> I know that RISC-V is starting to share the bindings. So really (part of) the
> documentation should be moved to common. Until then, I think it is misleading
> to add "is only implemented for ARM".

Yes you are right. Maybe Oleksii or Alejandro can fix this, moving this
file to common.

For this smaller patch series, I would remove the "is only implemented
for ARM".


> BTW, the spelling for should be "Arm" ;).

:-)
Re: [PATCH v3 2/5] xen/arm: dom0less: Add trap-unmapped-accesses
Posted by Edgar E. Iglesias 4 months, 4 weeks ago
On Mon, Jun 02, 2025 at 03:57:28PM -0700, Stefano Stabellini wrote:
> On Mon, 2 Jun 2025, Julien Grall wrote:
> > Hi,
> > 
> > On 02/06/2025 23:36, Stefano Stabellini wrote:
> > > On Fri, 30 May 2025, Edgar E. Iglesias wrote:
> > > > From: "Edgar E. Iglesias" <edgar.iglesias@amd.com>
> > > > 
> > > > Add the trap-unmapped-accesses per-domain fdt property.
> > > > 
> > > > Signed-off-by: Edgar E. Iglesias <edgar.iglesias@amd.com>
> > > > ---
> > > >   docs/misc/arm/device-tree/booting.txt | 10 ++++++++++
> > > >   xen/arch/arm/dom0less-build.c         |  9 ++++++++-
> > > >   2 files changed, 18 insertions(+), 1 deletion(-)
> > > > 
> > > > diff --git a/docs/misc/arm/device-tree/booting.txt
> > > > b/docs/misc/arm/device-tree/booting.txt
> > > > index 59fa96a82e..9add6440de 100644
> > > > --- a/docs/misc/arm/device-tree/booting.txt
> > > > +++ b/docs/misc/arm/device-tree/booting.txt
> > > > @@ -225,6 +225,16 @@ with the following properties:
> > > >       option is provided with a non zero value, but the platform doesn't
> > > > support
> > > >       SVE.
> > > >   +- trap-unmapped-accesses
> > > > +
> > > > +    Optional. An integer that configures handling of accesses to unmapped
> > > > +    address ranges.
> > > > +    If set to 0, guest accesses will read all bits as ones, e.g
> > > > 0xFFFFFFFF
> > > > +    for a 32bit access and writes will be ignored.
> > > > +    If set to 1, guest accesses will trap.
> > > > +
> > > > +    This option is only implemented for ARM where the default is 1.
> > > 
> > > Please expand it to: "This option is only implemented for ARM where the
> > > default is 1 when trap-unmapped-accesses is absent."
> > 
> > I am confused. The document is part of "docs/misc/arm" and some options like
> > "sve" are Arm specific. We don't mention this is Arm only because the
> > documention is Arm specific.
> > 
> > I know that RISC-V is starting to share the bindings. So really (part of) the
> > documentation should be moved to common. Until then, I think it is misleading
> > to add "is only implemented for ARM".
> 
> Yes you are right. Maybe Oleksii or Alejandro can fix this, moving this
> file to common.
> 
> For this smaller patch series, I would remove the "is only implemented
> for ARM".

Thanks, I've changed that line to:
"The default is 1 when trap-unmapped-accesses is absent."

Cheers,
Edgar