Hi Julien,
> On 7 Sep 2022, at 2:09 pm, Julien Grall <[email protected]> wrote:
>
> Hi Rahul
>
> On 06/09/2022 14:40, Rahul Singh wrote:
>> Introduce a new "xen,enhanced" dom0less property value "no-xenstore" to
>> disable xenstore interface for dom0less guests.
>> Signed-off-by: Rahul Singh <[email protected]>
>> ---
>> Changes in v4:
>> - Implement defines for dom0less features
>> Changes in v3:
>> - new patch in this version
>> ---
>> docs/misc/arm/device-tree/booting.txt | 4 ++++
>> xen/arch/arm/domain_build.c | 10 ++++++----
>> xen/arch/arm/include/asm/kernel.h | 23 +++++++++++++++++++++--
>> 3 files changed, 31 insertions(+), 6 deletions(-)
>> diff --git a/docs/misc/arm/device-tree/booting.txt
>> b/docs/misc/arm/device-tree/booting.txt
>> index 98253414b8..1b0dca1454 100644
>> --- a/docs/misc/arm/device-tree/booting.txt
>> +++ b/docs/misc/arm/device-tree/booting.txt
>> @@ -204,6 +204,10 @@ with the following properties:
>> - "disabled"
>> Xen PV interfaces are disabled.
>> + - no-xenstore
>> + Xen PV interfaces, including grant-table will be enabled but xenstore
>
> Please use "All default" in front. So it is clear that everything is enabled
> but xenstore.
Ack.
>
>> + will be disabled for the VM.
>> +
>> 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.
>> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
>> index 707e247f6a..0b164ef595 100644
>> --- a/xen/arch/arm/domain_build.c
>> +++ b/xen/arch/arm/domain_build.c
>> @@ -2891,7 +2891,7 @@ static int __init prepare_dtb_domU(struct domain *d,
>> struct kernel_info *kinfo)
>> goto err;
>> }
>> - if ( kinfo->dom0less_enhanced )
>> + if ( kinfo->dom0less_feature & DOM0LESS_ENHANCED_NO_XS )
>> {
>> ret = make_hypervisor_node(d, kinfo, addrcells, sizecells);
>> if ( ret )
>> @@ -3209,10 +3209,12 @@ static int __init construct_domU(struct domain *d,
>> (rc == 0 && !strcmp(dom0less_enhanced, "enabled")) )
>> {
>> if ( hardware_domain )
>> - kinfo.dom0less_enhanced = true;
>> + kinfo.dom0less_feature = DOM0LESS_ENHANCED;
>> else
>> - panic("Tried to use xen,enhanced without dom0\n");
>> + panic("At the moment, Xenstore support requires dom0 to be
>> present\n");
>> }
>> + else if ( rc == 0 && !strcmp(dom0less_enhanced, "no-xenstore") )
>> + kinfo.dom0less_feature = DOM0LESS_ENHANCED_NO_XS;
>> if ( vcpu_create(d, 0) == NULL )
>> return -ENOMEM;
>> @@ -3252,7 +3254,7 @@ static int __init construct_domU(struct domain *d,
>> if ( rc < 0 )
>> return rc;
>> - if ( kinfo.dom0less_enhanced )
>> + if ( kinfo.dom0less_feature & DOM0LESS_XENSTORE )
>> {
>> ASSERT(hardware_domain);
>> rc = alloc_xenstore_evtchn(d);
>> diff --git a/xen/arch/arm/include/asm/kernel.h
>> b/xen/arch/arm/include/asm/kernel.h
>> index c4dc039b54..ad240494ea 100644
>> --- a/xen/arch/arm/include/asm/kernel.h
>> +++ b/xen/arch/arm/include/asm/kernel.h
>> @@ -9,6 +9,25 @@
>> #include <xen/device_tree.h>
>> #include <asm/setup.h>
>> +/*
>> + * List of possible features for dom0less domUs
>> + *
>> + * DOM0LESS_ENHANCED_NO_XS: Notify the OS it is running on top of Xen. All
>> the
>> + * default features (excluding Xenstore) will be
>> + * available. Note that an OS *must* not rely on
>> the
>> + * availability of Xen features if this is not set.
>> + * DOM0LESS_XENSTORE: Xenstore will be enabled for the VM. This
>> feature
>> + * can't be enabled without the
>> + * DOM0LESS_ENHANCED_NO_XS.
>> + * DOM0LESS_ENHANCED: Notify the OS it is running on top of Xen. All
>> the
>> + * default features (including Xenstore) will be
>> + * available. Note that an OS *must* not rely on
>> the
>> + * availability of Xen features if this is not set.
>> + */
>> +#define DOM0LESS_ENHANCED_NO_XS BIT(0, U)
>> +#define DOM0LESS_XENSTORE BIT(1, U)
>> +#define DOM0LESS_ENHANCED (DOM0LESS_ENHANCED_NO_XS |
>> DOM0LESS_XENSTORE)
>> +
>> struct kernel_info {
>> #ifdef CONFIG_ARM_64
>> enum domain_type type;
>> @@ -36,8 +55,8 @@ struct kernel_info {
>> /* Enable pl011 emulation */
>> bool vpl011;
>> - /* Enable PV drivers */
>> - bool dom0less_enhanced;
>> + /* Enable/Disable PV drivers interface,grant table, evtchn or xenstore
>> */
>
> The part after "," is technically wrong because it also affects other
> interfaces. But I would drop it to avoid any stale comment (we may add new
> one in the futures).
Ok . I will remove and will comment like this:
/* Enable/Disable PV drivers interfaces */
Regards,
Rahul
