Hi Rahul,
On 01/09/2022 10:13, 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 <rahul.si...@arm.com>
---
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 | 3 +++
3 files changed, 14 insertions(+), 3 deletions(-)
diff --git a/docs/misc/arm/device-tree/booting.txt
b/docs/misc/arm/device-tree/booting.txt
index edef98e6d1..87f57f8889 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 for the VM but
+ xenstore will be disabled for the VM.
NIT: I would drop one of the "for the VM" as it seems to be redundant.
+
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 4b24261825..8dd9984225 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -3336,10 +3336,14 @@ static int __init construct_domU(struct domain *d,
(rc == 0 && !strcmp(dom0less_enhanced, "enabled")) )
{
if ( hardware_domain )
- kinfo.dom0less_enhanced = true;
+ kinfo.dom0less_xenstore = true;
else
- panic("Tried to use xen,enhanced without dom0\n");
+ panic("Tried to use xen,enhanced without dom0 without
no-xenstore\n");
This is a bit hard to parse. How about:
"At the moment, Xenstore support requires dom0 to be present"
}
+ else if ( rc == 0 && !strcmp(dom0less_enhanced, "no-xenstore") )
+ kinfo.dom0less_xenstore = false;
+
+ kinfo.dom0less_enhanced = true;
Wouldn't this now set dom0less_enhanced unconditionally?
if ( vcpu_create(d, 0) == NULL )
return -ENOMEM;
@@ -3379,7 +3383,7 @@ static int __init construct_domU(struct domain *d,
if ( rc < 0 )
return rc;
- if ( kinfo.dom0less_enhanced )
+ if ( kinfo.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..3d7fa94910 100644
--- a/xen/arch/arm/include/asm/kernel.h
+++ b/xen/arch/arm/include/asm/kernel.h
@@ -39,6 +39,9 @@ struct kernel_info {
/* Enable PV drivers */
bool dom0less_enhanced;
+ /* Enable Xenstore */
+ bool dom0less_xenstore;
+
AFAIU, it is not possible to have *_xenstore = true and *_enhanced =
false. I think it would be clearer if ``dom0less_enhanced`` is turned to
an enum with 3 values:
- None
- NOXENSTORE/BASIC
- FULLY_ENHANCED
If we want to be future proof, I would use a field 'flags' where
non-zero means enhanced. Each bit would indicate which features of Xen
is exposed.
Cheers,
--
Julien Grall