From: Stefano Stabellini <stefano.stabellini@xilinx.com>

Introduce a new "xen,enhanced" dom0less property to enable/disable PV
driver interfaces for dom0less guests. Currently only "enabled" and
"disabled" are supported property values (and empty). Leave the option
open to implement further possible values in the future (e.g.
"xenstore" to enable only xenstore.)

This patch only parses the property. Next patches will make use of it.

Signed-off-by: Stefano Stabellini <stefano.stabellini@xilinx.com>
CC: Julien Grall <julien@xen.org>
CC: Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>
CC: Bertrand Marquis <bertrand.marquis@arm.com>
---
 docs/misc/arm/device-tree/booting.txt | 18 ++++++++++++++++++
 xen/arch/arm/domain_build.c           |  5 +++++
 xen/arch/arm/include/asm/kernel.h     |  3 +++
 3 files changed, 26 insertions(+)

diff --git a/docs/misc/arm/device-tree/booting.txt b/docs/misc/arm/device-tree/booting.txt
index 71895663a4..38c29fb3d8 100644
--- a/docs/misc/arm/device-tree/booting.txt
+++ b/docs/misc/arm/device-tree/booting.txt
@@ -169,6 +169,24 @@ with the following properties:
     Please note that the SPI used for the virtual pl011 could clash with the
     physical SPI of a physical device assigned to the guest.
 
+- xen,enhanced
+
+    A string property. Possible property values are:
+
+    - "enabled" (or missing property value)
+    Xen PV interfaces, including grant-table and xenstore, will be
+    enabled for the VM.
+
+    - "disabled"
+    Xen PV interfaces are disabled.
+
+    If the xen,enhanced property is present with no value, it defaults
+    to "enabled". If the xen,enhanced property is not present, PV
+    interfaces are disabled.
+
+    In the future other possible property values might be added to
+    enable only selected interfaces.
+
 - nr_spis
 
     Optional. A 32-bit integer specifying the number of SPIs (Shared
diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 6931c022a2..96a94fa434 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -2963,6 +2963,7 @@ static int __init construct_domU(struct domain *d,
                                  const struct dt_device_node *node)
 {
     struct kernel_info kinfo = {};
+    const char *enhanced;
     int rc;
     u64 mem;
 
@@ -2978,6 +2979,10 @@ static int __init construct_domU(struct domain *d,
 
     kinfo.vpl011 = dt_property_read_bool(node, "vpl011");
 
+    rc = dt_property_read_string(node, "xen,enhanced", &enhanced);
+    if ( rc == -EILSEQ || (rc == 0 && !strcmp(enhanced, "enabled")) )
+        kinfo.enhanced = true;
+
     if ( vcpu_create(d, 0) == NULL )
         return -ENOMEM;
 
diff --git a/xen/arch/arm/include/asm/kernel.h b/xen/arch/arm/include/asm/kernel.h
index 874aa108a7..3275f7fbca 100644
--- a/xen/arch/arm/include/asm/kernel.h
+++ b/xen/arch/arm/include/asm/kernel.h
@@ -36,6 +36,9 @@ struct kernel_info {
     /* Enable pl011 emulation */
     bool vpl011;
 
+    /* Enable PV drivers */
+    bool enhanced;
+
     /* GIC phandle */
     uint32_t phandle_gic;
 
-- 
2.25.1

Hi Stefano,

Stefano Stabellini <sstabellini@kernel.org> writes:

> From: Stefano Stabellini <stefano.stabellini@xilinx.com>
>
> Introduce a new "xen,enhanced" dom0less property to enable/disable PV
> driver interfaces for dom0less guests. Currently only "enabled" and
> "disabled" are supported property values (and empty). Leave the option
> open to implement further possible values in the future (e.g.
> "xenstore" to enable only xenstore.)
>
> This patch only parses the property. Next patches will make use of it.
>
> Signed-off-by: Stefano Stabellini <stefano.stabellini@xilinx.com>
> CC: Julien Grall <julien@xen.org>
> CC: Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>
> CC: Bertrand Marquis <bertrand.marquis@arm.com>
> ---
>  docs/misc/arm/device-tree/booting.txt | 18 ++++++++++++++++++
>  xen/arch/arm/domain_build.c           |  5 +++++
>  xen/arch/arm/include/asm/kernel.h     |  3 +++
>  3 files changed, 26 insertions(+)
>
> diff --git a/docs/misc/arm/device-tree/booting.txt b/docs/misc/arm/device-tree/booting.txt
> index 71895663a4..38c29fb3d8 100644
> --- a/docs/misc/arm/device-tree/booting.txt
> +++ b/docs/misc/arm/device-tree/booting.txt
> @@ -169,6 +169,24 @@ with the following properties:
>      Please note that the SPI used for the virtual pl011 could clash with the
>      physical SPI of a physical device assigned to the guest.
>  
> +- xen,enhanced
> +
> +    A string property. Possible property values are:
> +
> +    - "enabled" (or missing property value)
> +    Xen PV interfaces, including grant-table and xenstore, will be
> +    enabled for the VM.
> +
> +    - "disabled"
> +    Xen PV interfaces are disabled.
> +
> +    If the xen,enhanced property is present with no value, it defaults
> +    to "enabled". If the xen,enhanced property is not present, PV
> +    interfaces are disabled.
> +
> +    In the future other possible property values might be added to
> +    enable only selected interfaces.
> +
>  - nr_spis
>  
>      Optional. A 32-bit integer specifying the number of SPIs (Shared
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index 6931c022a2..96a94fa434 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -2963,6 +2963,7 @@ static int __init construct_domU(struct domain *d,
>                                   const struct dt_device_node *node)
>  {
>      struct kernel_info kinfo = {};
> +    const char *enhanced;
>      int rc;
>      u64 mem;
>  
> @@ -2978,6 +2979,10 @@ static int __init construct_domU(struct domain *d,
>  
>      kinfo.vpl011 = dt_property_read_bool(node, "vpl011");
>  
> +    rc = dt_property_read_string(node, "xen,enhanced", &enhanced);
> +    if ( rc == -EILSEQ || (rc == 0 && !strcmp(enhanced, "enabled")) )

Are you sure you need to check for -EILSEQ?

From documentation for dt_property_read_string:

 * Returns 0 on
 * success, -EINVAL if the property does not exist, -ENODATA if property
 * doest not have value, and -EILSEQ if the string is not
 * null-terminated with the length of the property data.


So, for me it looks like you need to check for -ENODATA.

> +        kinfo.enhanced = true;
> +
>      if ( vcpu_create(d, 0) == NULL )
>          return -ENOMEM;
>  
> diff --git a/xen/arch/arm/include/asm/kernel.h b/xen/arch/arm/include/asm/kernel.h
> index 874aa108a7..3275f7fbca 100644
> --- a/xen/arch/arm/include/asm/kernel.h
> +++ b/xen/arch/arm/include/asm/kernel.h
> @@ -36,6 +36,9 @@ struct kernel_info {
>      /* Enable pl011 emulation */
>      bool vpl011;
>  
> +    /* Enable PV drivers */
> +    bool enhanced;

Maybe dom0less_enhanced will be better name?
Or what about dom0? Should it have this option enabled?

> +
>      /* GIC phandle */
>      uint32_t phandle_gic;


-- 
Volodymyr Babchuk at EPAM

On Tue, 11 Jan 2022, Volodymyr Babchuk wrote:
> Hi Stefano,
> 
> Stefano Stabellini <sstabellini@kernel.org> writes:
> 
> > From: Stefano Stabellini <stefano.stabellini@xilinx.com>
> >
> > Introduce a new "xen,enhanced" dom0less property to enable/disable PV
> > driver interfaces for dom0less guests. Currently only "enabled" and
> > "disabled" are supported property values (and empty). Leave the option
> > open to implement further possible values in the future (e.g.
> > "xenstore" to enable only xenstore.)
> >
> > This patch only parses the property. Next patches will make use of it.
> >
> > Signed-off-by: Stefano Stabellini <stefano.stabellini@xilinx.com>
> > CC: Julien Grall <julien@xen.org>
> > CC: Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>
> > CC: Bertrand Marquis <bertrand.marquis@arm.com>
> > ---
> >  docs/misc/arm/device-tree/booting.txt | 18 ++++++++++++++++++
> >  xen/arch/arm/domain_build.c           |  5 +++++
> >  xen/arch/arm/include/asm/kernel.h     |  3 +++
> >  3 files changed, 26 insertions(+)
> >
> > diff --git a/docs/misc/arm/device-tree/booting.txt b/docs/misc/arm/device-tree/booting.txt
> > index 71895663a4..38c29fb3d8 100644
> > --- a/docs/misc/arm/device-tree/booting.txt
> > +++ b/docs/misc/arm/device-tree/booting.txt
> > @@ -169,6 +169,24 @@ with the following properties:
> >      Please note that the SPI used for the virtual pl011 could clash with the
> >      physical SPI of a physical device assigned to the guest.
> >  
> > +- xen,enhanced
> > +
> > +    A string property. Possible property values are:
> > +
> > +    - "enabled" (or missing property value)
> > +    Xen PV interfaces, including grant-table and xenstore, will be
> > +    enabled for the VM.
> > +
> > +    - "disabled"
> > +    Xen PV interfaces are disabled.
> > +
> > +    If the xen,enhanced property is present with no value, it defaults
> > +    to "enabled". If the xen,enhanced property is not present, PV
> > +    interfaces are disabled.
> > +
> > +    In the future other possible property values might be added to
> > +    enable only selected interfaces.
> > +
> >  - nr_spis
> >  
> >      Optional. A 32-bit integer specifying the number of SPIs (Shared
> > diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> > index 6931c022a2..96a94fa434 100644
> > --- a/xen/arch/arm/domain_build.c
> > +++ b/xen/arch/arm/domain_build.c
> > @@ -2963,6 +2963,7 @@ static int __init construct_domU(struct domain *d,
> >                                   const struct dt_device_node *node)
> >  {
> >      struct kernel_info kinfo = {};
> > +    const char *enhanced;
> >      int rc;
> >      u64 mem;
> >  
> > @@ -2978,6 +2979,10 @@ static int __init construct_domU(struct domain *d,
> >  
> >      kinfo.vpl011 = dt_property_read_bool(node, "vpl011");
> >  
> > +    rc = dt_property_read_string(node, "xen,enhanced", &enhanced);
> > +    if ( rc == -EILSEQ || (rc == 0 && !strcmp(enhanced, "enabled")) )
> 
> Are you sure you need to check for -EILSEQ?
> 
> >From documentation for dt_property_read_string:
> 
>  * Returns 0 on
>  * success, -EINVAL if the property does not exist, -ENODATA if property
>  * doest not have value, and -EILSEQ if the string is not
>  * null-terminated with the length of the property data.
> 
> So, for me it looks like you need to check for -ENODATA.

That's interesting, I ran a few tests with:

  fdt set /chosen/domU0 xen,enhanced

and I keep getting:

  (XEN) DEBUG construct_domU 3009 rc=-84 val=<NULL>

And -84 is -EILSEQ.

So I'll check for both -EILSEQ and -ENODATA to be safe.

 
> 
> > +        kinfo.enhanced = true;
> > +
> >      if ( vcpu_create(d, 0) == NULL )
> >          return -ENOMEM;
> >  
> > diff --git a/xen/arch/arm/include/asm/kernel.h b/xen/arch/arm/include/asm/kernel.h
> > index 874aa108a7..3275f7fbca 100644
> > --- a/xen/arch/arm/include/asm/kernel.h
> > +++ b/xen/arch/arm/include/asm/kernel.h
> > @@ -36,6 +36,9 @@ struct kernel_info {
> >      /* Enable pl011 emulation */
> >      bool vpl011;
> >  
> > +    /* Enable PV drivers */
> > +    bool enhanced;
> 
> Maybe dom0less_enhanced will be better name?

I am OK with that and maybe you are right, it is clearer. I'll keep the
device tree option as "xen,enhanced" unless you meant to also change
that too.


> Or what about dom0? Should it have this option enabled?

Yes, I think it makes sense to set it to true for dom0 too for
consistency.