* [PATCH v4 01/11] xen: introduce internal CDF_xxx flags for domain creation
2021-12-20 5:21 [PATCH v4 00/11] direct-map memory map Penny Zheng
@ 2021-12-20 5:21 ` Penny Zheng
2022-01-07 14:14 ` Jan Beulich
2021-12-20 5:21 ` [PATCH v4 02/11] xen: introduce CDF_directmap Penny Zheng
` (10 subsequent siblings)
11 siblings, 1 reply; 20+ messages in thread
From: Penny Zheng @ 2021-12-20 5:21 UTC (permalink / raw)
To: xen-devel, sstabellini, julien; +Cc: Bertrand.Marquis, Wei.Chen
From: Stefano Stabellini <stefano.stabellini@xilinx.com>
We are passing an internal-only boolean flag at domain creation to
specify whether we want the domain to be privileged (i.e. dom0) or
not. Another flag will be introduced later in this series.
This commit extends original "boolean" to an "unsigned int" covering both
the existing "is_priv" and our new "directmap", which will be introduced later.
To make visible the relationship, we name the respective constants CDF_xxx
(with no XEN_DOMCTL_ prefix) to represent the difference with the public
constants XEN_DOMCTL_CDF_xxx.
Allocate bit 0 as CDF_privileged: whether a domain is privileged or not.
Signed-off-by: Stefano Stabellini <stefano.stabellini@xilinx.com>
Signed-off-by: Penny Zheng <penny.zheng@arm.com>
---
v4 changes:
- new commit
---
xen/arch/arm/domain_build.c | 4 ++--
xen/arch/x86/setup.c | 2 +-
xen/common/domain.c | 10 +++++-----
xen/common/sched/core.c | 2 +-
xen/include/xen/domain.h | 4 ++++
xen/include/xen/sched.h | 2 +-
6 files changed, 14 insertions(+), 10 deletions(-)
diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 6931c022a2..0fab8604de 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -3058,7 +3058,7 @@ void __init create_domUs(void)
* very important to use the pre-increment operator to call
* domain_create() with a domid > 0. (domid == 0 is reserved for Dom0)
*/
- d = domain_create(++max_init_domid, &d_cfg, false);
+ d = domain_create(++max_init_domid, &d_cfg, 0);
if ( IS_ERR(d) )
panic("Error creating domain %s\n", dt_node_name(node));
@@ -3160,7 +3160,7 @@ void __init create_dom0(void)
if ( iommu_enabled )
dom0_cfg.flags |= XEN_DOMCTL_CDF_iommu;
- dom0 = domain_create(0, &dom0_cfg, true);
+ dom0 = domain_create(0, &dom0_cfg, CDF_privileged);
if ( IS_ERR(dom0) || (alloc_dom0_vcpu0(dom0) == NULL) )
panic("Error creating domain 0\n");
diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index e716005145..a14271488c 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -789,7 +789,7 @@ static struct domain *__init create_dom0(const module_t *image,
/* Create initial domain. Not d0 for pvshim. */
domid = get_initial_domain_id();
- d = domain_create(domid, &dom0_cfg, !pv_shim);
+ d = domain_create(domid, &dom0_cfg, pv_shim ? 0 : CDF_privileged);
if ( IS_ERR(d) )
panic("Error creating d%u: %ld\n", domid, PTR_ERR(d));
diff --git a/xen/common/domain.c b/xen/common/domain.c
index 2048ebad86..023c89c0ea 100644
--- a/xen/common/domain.c
+++ b/xen/common/domain.c
@@ -552,7 +552,7 @@ static int sanitise_domain_config(struct xen_domctl_createdomain *config)
struct domain *domain_create(domid_t domid,
struct xen_domctl_createdomain *config,
- bool is_priv)
+ const unsigned int flags)
{
struct domain *d, **pd, *old_hwdom = NULL;
enum { INIT_watchdog = 1u<<1,
@@ -578,7 +578,7 @@ struct domain *domain_create(domid_t domid,
}
/* Sort out our idea of is_control_domain(). */
- d->is_privileged = is_priv;
+ d->is_privileged = flags & CDF_privileged;
/* Sort out our idea of is_hardware_domain(). */
if ( domid == 0 || domid == hardware_domid )
@@ -772,7 +772,7 @@ void __init setup_system_domains(void)
* Hidden PCI devices will also be associated with this domain
* (but be [partly] controlled by Dom0 nevertheless).
*/
- dom_xen = domain_create(DOMID_XEN, NULL, false);
+ dom_xen = domain_create(DOMID_XEN, NULL, 0);
if ( IS_ERR(dom_xen) )
panic("Failed to create d[XEN]: %ld\n", PTR_ERR(dom_xen));
@@ -782,7 +782,7 @@ void __init setup_system_domains(void)
* array. Mappings occur at the priv of the caller.
* Quarantined PCI devices will be associated with this domain.
*/
- dom_io = domain_create(DOMID_IO, NULL, false);
+ dom_io = domain_create(DOMID_IO, NULL, 0);
if ( IS_ERR(dom_io) )
panic("Failed to create d[IO]: %ld\n", PTR_ERR(dom_io));
@@ -791,7 +791,7 @@ void __init setup_system_domains(void)
* Initialise our COW domain.
* This domain owns sharable pages.
*/
- dom_cow = domain_create(DOMID_COW, NULL, false);
+ dom_cow = domain_create(DOMID_COW, NULL, 0);
if ( IS_ERR(dom_cow) )
panic("Failed to create d[COW]: %ld\n", PTR_ERR(dom_cow));
#endif
diff --git a/xen/common/sched/core.c b/xen/common/sched/core.c
index 8f4b1ca10d..f5c819349b 100644
--- a/xen/common/sched/core.c
+++ b/xen/common/sched/core.c
@@ -3021,7 +3021,7 @@ void __init scheduler_init(void)
sched_ratelimit_us = SCHED_DEFAULT_RATELIMIT_US;
}
- idle_domain = domain_create(DOMID_IDLE, NULL, false);
+ idle_domain = domain_create(DOMID_IDLE, NULL, 0);
BUG_ON(IS_ERR(idle_domain));
BUG_ON(nr_cpu_ids > ARRAY_SIZE(idle_vcpu));
idle_domain->vcpu = idle_vcpu;
diff --git a/xen/include/xen/domain.h b/xen/include/xen/domain.h
index 160c8dbdab..cfb0b47f13 100644
--- a/xen/include/xen/domain.h
+++ b/xen/include/xen/domain.h
@@ -28,6 +28,10 @@ void getdomaininfo(struct domain *d, struct xen_domctl_getdomaininfo *info);
void arch_get_domain_info(const struct domain *d,
struct xen_domctl_getdomaininfo *info);
+/* CDF_* constant. Internal flags for domain creation. */
+/* Is this a privileged domain? */
+#define CDF_privileged (1U << 0)
+
/*
* Arch-specifics.
*/
diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
index 37f78cc4c4..972877bc28 100644
--- a/xen/include/xen/sched.h
+++ b/xen/include/xen/sched.h
@@ -665,7 +665,7 @@ int arch_sanitise_domain_config(struct xen_domctl_createdomain *config);
*/
struct domain *domain_create(domid_t domid,
struct xen_domctl_createdomain *config,
- bool is_priv);
+ const unsigned int flags);
/*
* rcu_lock_domain_by_id() is more efficient than get_domain_by_id().
--
2.25.1
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH v4 01/11] xen: introduce internal CDF_xxx flags for domain creation
2021-12-20 5:21 ` [PATCH v4 01/11] xen: introduce internal CDF_xxx flags for domain creation Penny Zheng
@ 2022-01-07 14:14 ` Jan Beulich
0 siblings, 0 replies; 20+ messages in thread
From: Jan Beulich @ 2022-01-07 14:14 UTC (permalink / raw)
To: Penny Zheng; +Cc: Bertrand.Marquis, Wei.Chen, xen-devel, sstabellini, julien
On 20.12.2021 06:21, Penny Zheng wrote:
> From: Stefano Stabellini <stefano.stabellini@xilinx.com>
>
> We are passing an internal-only boolean flag at domain creation to
> specify whether we want the domain to be privileged (i.e. dom0) or
> not. Another flag will be introduced later in this series.
>
> This commit extends original "boolean" to an "unsigned int" covering both
> the existing "is_priv" and our new "directmap", which will be introduced later.
>
> To make visible the relationship, we name the respective constants CDF_xxx
> (with no XEN_DOMCTL_ prefix) to represent the difference with the public
> constants XEN_DOMCTL_CDF_xxx.
>
> Allocate bit 0 as CDF_privileged: whether a domain is privileged or not.
>
> Signed-off-by: Stefano Stabellini <stefano.stabellini@xilinx.com>
> Signed-off-by: Penny Zheng <penny.zheng@arm.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
with one remark:
> --- a/xen/common/domain.c
> +++ b/xen/common/domain.c
> @@ -552,7 +552,7 @@ static int sanitise_domain_config(struct xen_domctl_createdomain *config)
>
> struct domain *domain_create(domid_t domid,
> struct xen_domctl_createdomain *config,
> - bool is_priv)
> + const unsigned int flags)
We don't normally use const like this, so I'd suggest to drop it here
and ...
> --- a/xen/include/xen/sched.h
> +++ b/xen/include/xen/sched.h
> @@ -665,7 +665,7 @@ int arch_sanitise_domain_config(struct xen_domctl_createdomain *config);
> */
> struct domain *domain_create(domid_t domid,
> struct xen_domctl_createdomain *config,
> - bool is_priv);
> + const unsigned int flags);
... here.
Jan
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v4 02/11] xen: introduce CDF_directmap
2021-12-20 5:21 [PATCH v4 00/11] direct-map memory map Penny Zheng
2021-12-20 5:21 ` [PATCH v4 01/11] xen: introduce internal CDF_xxx flags for domain creation Penny Zheng
@ 2021-12-20 5:21 ` Penny Zheng
2022-01-07 14:22 ` Jan Beulich
2021-12-20 5:21 ` [PATCH v4 03/11] xen/arm: avoid setting XEN_DOMCTL_CDF_iommu when IOMMU off Penny Zheng
` (9 subsequent siblings)
11 siblings, 1 reply; 20+ messages in thread
From: Penny Zheng @ 2021-12-20 5:21 UTC (permalink / raw)
To: xen-devel, sstabellini, julien; +Cc: Bertrand.Marquis, Wei.Chen
From: Stefano Stabellini <stefano.stabellini@xilinx.com>
This commit introduces a new arm-specific flag CDF_directmap to specify
that a domain should have its memory direct-map(guest physical address
== physical address) at domain creation.
Also, add a directmap flag under struct arch_domain and use it to
reimplement is_domain_direct_mapped.
For now, direct-map is only available when statically allocated memory is
used for the domain, that is, "xen,static-mem" must be also defined in the
domain configuration.
Signed-off-by: Stefano Stabellini <stefano.stabellini@xilinx.com>
Signed-off-by: Penny Zheng <penny.zheng@arm.com>
---
v2 changes
- remove the introduce of internal flag
- remove flag direct_map since we already store this flag in d->options
- Refine is_domain_direct_mapped to check whether the flag
XEN_DOMCTL_CDF_directmap is set
- reword "1:1 direct-map" to just "direct-map"
---
v3 changes
- move flag back to xen/include/xen/domain.h, to let it be only available for
domain created by XEN.
- name it with extra "INTERNAL" and add comments to warn developers not
to accidently use its bitfield when introducing new XEN_DOMCTL_CDF_xxx flag.
- reject this flag in x86'es arch_sanitise_domain_config()
---
v4 changes
- introduce new internal flag CDF_directmap
- add a directmap flag under struct arch_domain and use it to
reimplement is_domain_direct_mapped.
- expand arch_domain_create to include internal-only parameter "const unsigned
int flags" for domain creation
---
docs/misc/arm/device-tree/booting.txt | 6 ++++++
xen/arch/arm/domain.c | 5 ++++-
xen/arch/arm/domain_build.c | 14 ++++++++++++--
xen/arch/arm/include/asm/domain.h | 5 +++--
xen/arch/x86/domain.c | 3 ++-
xen/common/domain.c | 2 +-
xen/include/xen/domain.h | 5 ++++-
7 files changed, 32 insertions(+), 8 deletions(-)
diff --git a/docs/misc/arm/device-tree/booting.txt b/docs/misc/arm/device-tree/booting.txt
index 71895663a4..a94125394e 100644
--- a/docs/misc/arm/device-tree/booting.txt
+++ b/docs/misc/arm/device-tree/booting.txt
@@ -182,6 +182,12 @@ with the following properties:
Both #address-cells and #size-cells need to be specified because
both sub-nodes (described shortly) have reg properties.
+- direct-map
+
+ Only available when statically allocated memory is used for the domain.
+ An empty property to request the memory of the domain to be
+ direct-map (guest physical address == physical address).
+
Under the "xen,domain" compatible node, one or more sub-nodes are present
for the DomU kernel and ramdisk.
diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
index 92a6c509e5..fa18dfa544 100644
--- a/xen/arch/arm/domain.c
+++ b/xen/arch/arm/domain.c
@@ -692,7 +692,8 @@ int arch_sanitise_domain_config(struct xen_domctl_createdomain *config)
}
int arch_domain_create(struct domain *d,
- struct xen_domctl_createdomain *config)
+ struct xen_domctl_createdomain *config,
+ const unsigned int flags)
{
int rc, count = 0;
@@ -708,6 +709,8 @@ int arch_domain_create(struct domain *d,
ioreq_domain_init(d);
#endif
+ d->arch.directmap = flags & CDF_directmap;
+
/* p2m_init relies on some value initialized by the IOMMU subsystem */
if ( (rc = iommu_domain_init(d, config->iommu_opts)) != 0 )
goto fail;
diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 0fab8604de..9b1a5e38d3 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -3029,10 +3029,20 @@ void __init create_domUs(void)
.max_maptrack_frames = -1,
.grant_opts = XEN_DOMCTL_GRANT_version(opt_gnttab_max_version),
};
+ unsigned int flags = 0U;
if ( !dt_device_is_compatible(node, "xen,domain") )
continue;
+ if ( dt_property_read_bool(node, "direct-map") )
+ {
+ if ( !IS_ENABLED(CONFIG_STATIC_MEMORY) )
+ panic("direct-map is not valid for domain %s without CONFIG_STATIC_MEMORY\n",
+ dt_node_name(node));
+
+ flags |= CDF_directmap;
+ }
+
if ( !dt_property_read_u32(node, "cpus", &d_cfg.max_vcpus) )
panic("Missing property 'cpus' for domain %s\n",
dt_node_name(node));
@@ -3058,7 +3068,7 @@ void __init create_domUs(void)
* very important to use the pre-increment operator to call
* domain_create() with a domid > 0. (domid == 0 is reserved for Dom0)
*/
- d = domain_create(++max_init_domid, &d_cfg, 0);
+ d = domain_create(++max_init_domid, &d_cfg, flags);
if ( IS_ERR(d) )
panic("Error creating domain %s\n", dt_node_name(node));
@@ -3160,7 +3170,7 @@ void __init create_dom0(void)
if ( iommu_enabled )
dom0_cfg.flags |= XEN_DOMCTL_CDF_iommu;
- dom0 = domain_create(0, &dom0_cfg, CDF_privileged);
+ dom0 = domain_create(0, &dom0_cfg, CDF_privileged | CDF_directmap);
if ( IS_ERR(dom0) || (alloc_dom0_vcpu0(dom0) == NULL) )
panic("Error creating domain 0\n");
diff --git a/xen/arch/arm/include/asm/domain.h b/xen/arch/arm/include/asm/domain.h
index 9b3647587a..cb37ce89ec 100644
--- a/xen/arch/arm/include/asm/domain.h
+++ b/xen/arch/arm/include/asm/domain.h
@@ -29,8 +29,7 @@ enum domain_type {
#define is_64bit_domain(d) (0)
#endif
-/* The hardware domain has always its memory direct mapped. */
-#define is_domain_direct_mapped(d) is_hardware_domain(d)
+#define is_domain_direct_mapped(d) (d->arch.directmap)
struct vtimer {
struct vcpu *v;
@@ -89,6 +88,8 @@ struct arch_domain
#ifdef CONFIG_TEE
void *tee;
#endif
+
+ bool directmap;
} __cacheline_aligned;
struct arch_vcpu
diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
index ef1812dc14..f6de7a9697 100644
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -722,7 +722,8 @@ static bool emulation_flags_ok(const struct domain *d, uint32_t emflags)
}
int arch_domain_create(struct domain *d,
- struct xen_domctl_createdomain *config)
+ struct xen_domctl_createdomain *config,
+ const unsigned int flags)
{
bool paging_initialised = false;
uint32_t emflags;
diff --git a/xen/common/domain.c b/xen/common/domain.c
index 023c89c0ea..c9bcd77e00 100644
--- a/xen/common/domain.c
+++ b/xen/common/domain.c
@@ -659,7 +659,7 @@ struct domain *domain_create(domid_t domid,
radix_tree_init(&d->pirq_tree);
}
- if ( (err = arch_domain_create(d, config)) != 0 )
+ if ( (err = arch_domain_create(d, config, flags)) != 0 )
goto fail;
init_status |= INIT_arch;
diff --git a/xen/include/xen/domain.h b/xen/include/xen/domain.h
index cfb0b47f13..2f1e172957 100644
--- a/xen/include/xen/domain.h
+++ b/xen/include/xen/domain.h
@@ -31,6 +31,8 @@ void arch_get_domain_info(const struct domain *d,
/* CDF_* constant. Internal flags for domain creation. */
/* Is this a privileged domain? */
#define CDF_privileged (1U << 0)
+/* Should domain memory be directly mapped? */
+#define CDF_directmap (1U << 1)
/*
* Arch-specifics.
@@ -65,7 +67,8 @@ int map_vcpu_info(struct vcpu *v, unsigned long gfn, unsigned offset);
void unmap_vcpu_info(struct vcpu *v);
int arch_domain_create(struct domain *d,
- struct xen_domctl_createdomain *config);
+ struct xen_domctl_createdomain *config,
+ const unsigned int flags);
void arch_domain_destroy(struct domain *d);
--
2.25.1
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH v4 02/11] xen: introduce CDF_directmap
2021-12-20 5:21 ` [PATCH v4 02/11] xen: introduce CDF_directmap Penny Zheng
@ 2022-01-07 14:22 ` Jan Beulich
2022-01-21 9:15 ` Penny Zheng
0 siblings, 1 reply; 20+ messages in thread
From: Jan Beulich @ 2022-01-07 14:22 UTC (permalink / raw)
To: Penny Zheng; +Cc: Bertrand.Marquis, Wei.Chen, xen-devel, sstabellini, julien
On 20.12.2021 06:21, Penny Zheng wrote:
> From: Stefano Stabellini <stefano.stabellini@xilinx.com>
>
> This commit introduces a new arm-specific flag CDF_directmap to specify
> that a domain should have its memory direct-map(guest physical address
> == physical address) at domain creation.
>
> Also, add a directmap flag under struct arch_domain and use it to
> reimplement is_domain_direct_mapped.
>
> For now, direct-map is only available when statically allocated memory is
> used for the domain, that is, "xen,static-mem" must be also defined in the
> domain configuration.
>
> Signed-off-by: Stefano Stabellini <stefano.stabellini@xilinx.com>
> Signed-off-by: Penny Zheng <penny.zheng@arm.com>
Non-Arm parts
Acked-by: Jan Beulich <jbeulich@suse.com>
However, ...
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -3029,10 +3029,20 @@ void __init create_domUs(void)
> .max_maptrack_frames = -1,
> .grant_opts = XEN_DOMCTL_GRANT_version(opt_gnttab_max_version),
> };
> + unsigned int flags = 0U;
Nit: No real need for a U suffix here.
> if ( !dt_device_is_compatible(node, "xen,domain") )
> continue;
>
> + if ( dt_property_read_bool(node, "direct-map") )
> + {
> + if ( !IS_ENABLED(CONFIG_STATIC_MEMORY) )
Isn't this too lax a check? I didn't find any other check of this
property, so the use of static memory must be keyed to something
else. Hence it's not sufficient that static memory support is
enabled in the build.
> @@ -65,7 +67,8 @@ int map_vcpu_info(struct vcpu *v, unsigned long gfn, unsigned offset);
> void unmap_vcpu_info(struct vcpu *v);
>
> int arch_domain_create(struct domain *d,
> - struct xen_domctl_createdomain *config);
> + struct xen_domctl_createdomain *config,
> + const unsigned int flags);
Same comment as for the earlier patch regarding the const here.
Jan
^ permalink raw reply [flat|nested] 20+ messages in thread* RE: [PATCH v4 02/11] xen: introduce CDF_directmap
2022-01-07 14:22 ` Jan Beulich
@ 2022-01-21 9:15 ` Penny Zheng
0 siblings, 0 replies; 20+ messages in thread
From: Penny Zheng @ 2022-01-21 9:15 UTC (permalink / raw)
To: Jan Beulich; +Cc: Bertrand Marquis, Wei Chen, xen-devel, sstabellini, julien
Hi Jan
Sorry for the late reply.
> -----Original Message-----
> From: Jan Beulich <jbeulich@suse.com>
> Sent: Friday, January 7, 2022 10:22 PM
> To: Penny Zheng <Penny.Zheng@arm.com>
> Cc: Bertrand Marquis <Bertrand.Marquis@arm.com>; Wei Chen
> <Wei.Chen@arm.com>; xen-devel@lists.xenproject.org;
> sstabellini@kernel.org; julien@xen.org
> Subject: Re: [PATCH v4 02/11] xen: introduce CDF_directmap
>
> On 20.12.2021 06:21, Penny Zheng wrote:
> > From: Stefano Stabellini <stefano.stabellini@xilinx.com>
> >
> > This commit introduces a new arm-specific flag CDF_directmap to
> > specify that a domain should have its memory direct-map(guest physical
> > address == physical address) at domain creation.
> >
> > Also, add a directmap flag under struct arch_domain and use it to
> > reimplement is_domain_direct_mapped.
> >
> > For now, direct-map is only available when statically allocated memory
> > is used for the domain, that is, "xen,static-mem" must be also defined
> > in the domain configuration.
> >
> > Signed-off-by: Stefano Stabellini <stefano.stabellini@xilinx.com>
> > Signed-off-by: Penny Zheng <penny.zheng@arm.com>
>
> Non-Arm parts
> Acked-by: Jan Beulich <jbeulich@suse.com> However, ...
>
> > --- a/xen/arch/arm/domain_build.c
> > +++ b/xen/arch/arm/domain_build.c
> > @@ -3029,10 +3029,20 @@ void __init create_domUs(void)
> > .max_maptrack_frames = -1,
> > .grant_opts =
> XEN_DOMCTL_GRANT_version(opt_gnttab_max_version),
> > };
> > + unsigned int flags = 0U;
>
> Nit: No real need for a U suffix here.
>
> > if ( !dt_device_is_compatible(node, "xen,domain") )
> > continue;
> >
> > + if ( dt_property_read_bool(node, "direct-map") )
> > + {
> > + if ( !IS_ENABLED(CONFIG_STATIC_MEMORY) )
>
> Isn't this too lax a check? I didn't find any other check of this property, so the
> use of static memory must be keyed to something else. Hence it's not sufficient
> that static memory support is enabled in the build.
>
Ok.
I'll change the check to " if ( !IS_ENABLED(CONFIG_STATIC_MEMORY) || !dt_find_property(node, "xen,static-mem", NULL) ) "
to make the check a bit more strict.
> > @@ -65,7 +67,8 @@ int map_vcpu_info(struct vcpu *v, unsigned long gfn,
> > unsigned offset); void unmap_vcpu_info(struct vcpu *v);
> >
> > int arch_domain_create(struct domain *d,
> > - struct xen_domctl_createdomain *config);
> > + struct xen_domctl_createdomain *config,
> > + const unsigned int flags);
>
> Same comment as for the earlier patch regarding the const here.
>
> Jan
Many thanks,
Penny Zheng
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v4 03/11] xen/arm: avoid setting XEN_DOMCTL_CDF_iommu when IOMMU off
2021-12-20 5:21 [PATCH v4 00/11] direct-map memory map Penny Zheng
2021-12-20 5:21 ` [PATCH v4 01/11] xen: introduce internal CDF_xxx flags for domain creation Penny Zheng
2021-12-20 5:21 ` [PATCH v4 02/11] xen: introduce CDF_directmap Penny Zheng
@ 2021-12-20 5:21 ` Penny Zheng
2021-12-20 5:21 ` [PATCH v4 04/11] xen/arm: introduce new helper parse_static_mem_prop and acquire_static_memory_bank Penny Zheng
` (8 subsequent siblings)
11 siblings, 0 replies; 20+ messages in thread
From: Penny Zheng @ 2021-12-20 5:21 UTC (permalink / raw)
To: xen-devel, sstabellini, julien; +Cc: Bertrand.Marquis, Wei.Chen
From: Stefano Stabellini <stefano.stabellini@xilinx.com>
When IOMMU is absent or shall not be used (trusted domain, performance,
hardware limitation, ..., etc), in which cases this commit avoids setting
XEN_DOMCTL_CDF_iommu to make those user cases possible and prevent failure
later during device assignment.
Signed-off-by: Stefano Stabellini <stefano.stabellini@xilinx.com>
Signed-off-by: Penny Zheng <penny.zheng@arm.com>
---
v3 changes:
- new commit, split from the original "[PATCH v2 2/6] xen/arm: introduce
direct-map for domUs"
---
v4 changes
- explain briefly in the commit message why we want to do device assignment
without IOMMU.
---
xen/arch/arm/domain_build.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 9b1a5e38d3..5a106a977c 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -3047,7 +3047,8 @@ void __init create_domUs(void)
panic("Missing property 'cpus' for domain %s\n",
dt_node_name(node));
- if ( dt_find_compatible_node(node, NULL, "multiboot,device-tree") )
+ if ( dt_find_compatible_node(node, NULL, "multiboot,device-tree") &&
+ iommu_enabled )
d_cfg.flags |= XEN_DOMCTL_CDF_iommu;
if ( !dt_property_read_u32(node, "nr_spis", &d_cfg.arch.nr_spis) )
--
2.25.1
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH v4 04/11] xen/arm: introduce new helper parse_static_mem_prop and acquire_static_memory_bank
2021-12-20 5:21 [PATCH v4 00/11] direct-map memory map Penny Zheng
` (2 preceding siblings ...)
2021-12-20 5:21 ` [PATCH v4 03/11] xen/arm: avoid setting XEN_DOMCTL_CDF_iommu when IOMMU off Penny Zheng
@ 2021-12-20 5:21 ` Penny Zheng
2022-01-13 22:45 ` Stefano Stabellini
2021-12-20 5:21 ` [PATCH v4 05/11] xen/arm: introduce direct-map for domUs Penny Zheng
` (7 subsequent siblings)
11 siblings, 1 reply; 20+ messages in thread
From: Penny Zheng @ 2021-12-20 5:21 UTC (permalink / raw)
To: xen-devel, sstabellini, julien; +Cc: Bertrand.Marquis, Wei.Chen
Later, we will introduce allocate_static_memory_11 for allocating static
memory for direct-map domains, and it will share a lot common codes with
the existing allocate_static_memory.
In order not to bring a lot of duplicate codes, and also to make the whole
code more readable, this commit extracts common codes into two new helpers
parse_static_mem_prop and acquire_static_memory_bank.
Signed-off-by: Penny Zheng <penny.zheng@arm.com>
---
v3 changes:
- new commit to move the split off of the code outside in a separate patch
---
v4 changes:
- use mfn_eq() instead, because it is the only value used to indicate
there is an error and this is more lightweight than mfn_valid()
---
xen/arch/arm/domain_build.c | 100 +++++++++++++++++++++++-------------
1 file changed, 64 insertions(+), 36 deletions(-)
diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 5a106a977c..9206ec908d 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -509,12 +509,69 @@ static bool __init append_static_memory_to_bank(struct domain *d,
return true;
}
+static mfn_t __init acquire_static_memory_bank(struct domain *d,
+ const __be32 **cell,
+ u32 addr_cells, u32 size_cells,
+ paddr_t *pbase, paddr_t *psize)
+{
+ mfn_t smfn;
+ int res;
+
+ device_tree_get_reg(cell, addr_cells, size_cells, pbase, psize);
+ ASSERT(IS_ALIGNED(*pbase, PAGE_SIZE) && IS_ALIGNED(*psize, PAGE_SIZE));
+ if ( PFN_DOWN(*psize) > UINT_MAX )
+ {
+ printk(XENLOG_ERR "%pd: static memory size too large: %#"PRIpaddr,
+ d, *psize);
+ return INVALID_MFN;
+ }
+
+ smfn = maddr_to_mfn(*pbase);
+ res = acquire_domstatic_pages(d, smfn, PFN_DOWN(*psize), 0);
+ if ( res )
+ {
+ printk(XENLOG_ERR
+ "%pd: failed to acquire static memory: %d.\n", d, res);
+ return INVALID_MFN;
+ }
+
+ return smfn;
+}
+
+static int __init parse_static_mem_prop(const struct dt_device_node *node,
+ u32 *addr_cells, u32 *size_cells,
+ int *length, const __be32 **cell)
+{
+ const struct dt_property *prop;
+
+ prop = dt_find_property(node, "xen,static-mem", NULL);
+ if ( !dt_property_read_u32(node, "#xen,static-mem-address-cells",
+ addr_cells) )
+ {
+ printk(XENLOG_ERR
+ "failed to read \"#xen,static-mem-address-cells\".\n");
+ return -EINVAL;
+ }
+
+ if ( !dt_property_read_u32(node, "#xen,static-mem-size-cells",
+ size_cells) )
+ {
+ printk(XENLOG_ERR
+ "failed to read \"#xen,static-mem-size-cells\".\n");
+ return -EINVAL;
+ }
+
+ *cell = (const __be32 *)prop->value;
+ *length = prop->length;
+
+ return 0;
+}
+
/* Allocate memory from static memory as RAM for one specific domain d. */
static void __init allocate_static_memory(struct domain *d,
struct kernel_info *kinfo,
const struct dt_device_node *node)
{
- const struct dt_property *prop;
u32 addr_cells, size_cells, reg_cells;
unsigned int nr_banks, gbank, bank = 0;
const uint64_t rambase[] = GUEST_RAM_BANK_BASES;
@@ -523,24 +580,10 @@ static void __init allocate_static_memory(struct domain *d,
u64 tot_size = 0;
paddr_t pbase, psize, gsize;
mfn_t smfn;
- int res;
-
- prop = dt_find_property(node, "xen,static-mem", NULL);
- if ( !dt_property_read_u32(node, "#xen,static-mem-address-cells",
- &addr_cells) )
- {
- printk(XENLOG_ERR
- "%pd: failed to read \"#xen,static-mem-address-cells\".\n", d);
- goto fail;
- }
+ int length;
- if ( !dt_property_read_u32(node, "#xen,static-mem-size-cells",
- &size_cells) )
- {
- printk(XENLOG_ERR
- "%pd: failed to read \"#xen,static-mem-size-cells\".\n", d);
+ if ( parse_static_mem_prop(node, &addr_cells, &size_cells, &length, &cell) )
goto fail;
- }
reg_cells = addr_cells + size_cells;
/*
@@ -551,29 +594,14 @@ static void __init allocate_static_memory(struct domain *d,
gbank = 0;
gsize = ramsize[gbank];
kinfo->mem.bank[gbank].start = rambase[gbank];
-
- cell = (const __be32 *)prop->value;
- nr_banks = (prop->length) / (reg_cells * sizeof (u32));
+ nr_banks = length / (reg_cells * sizeof (u32));
for ( ; bank < nr_banks; bank++ )
{
- device_tree_get_reg(&cell, addr_cells, size_cells, &pbase, &psize);
- ASSERT(IS_ALIGNED(pbase, PAGE_SIZE) && IS_ALIGNED(psize, PAGE_SIZE));
-
- if ( PFN_DOWN(psize) > UINT_MAX )
- {
- printk(XENLOG_ERR "%pd: static memory size too large: %#"PRIpaddr,
- d, psize);
+ smfn = acquire_static_memory_bank(d, &cell, addr_cells, size_cells,
+ &pbase, &psize);
+ if ( mfn_eq(smfn, INVALID_MFN) )
goto fail;
- }
- smfn = maddr_to_mfn(pbase);
- res = acquire_domstatic_pages(d, smfn, PFN_DOWN(psize), 0);
- if ( res )
- {
- printk(XENLOG_ERR
- "%pd: failed to acquire static memory: %d.\n", d, res);
- goto fail;
- }
printk(XENLOG_INFO "%pd: STATIC BANK[%u] %#"PRIpaddr"-%#"PRIpaddr"\n",
d, bank, pbase, pbase + psize);
--
2.25.1
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH v4 04/11] xen/arm: introduce new helper parse_static_mem_prop and acquire_static_memory_bank
2021-12-20 5:21 ` [PATCH v4 04/11] xen/arm: introduce new helper parse_static_mem_prop and acquire_static_memory_bank Penny Zheng
@ 2022-01-13 22:45 ` Stefano Stabellini
0 siblings, 0 replies; 20+ messages in thread
From: Stefano Stabellini @ 2022-01-13 22:45 UTC (permalink / raw)
To: Penny Zheng; +Cc: xen-devel, sstabellini, julien, Bertrand.Marquis, Wei.Chen
On Mon, 20 Dec 2021, Penny Zheng wrote:
> Later, we will introduce allocate_static_memory_11 for allocating static
> memory for direct-map domains, and it will share a lot common codes with
> the existing allocate_static_memory.
>
> In order not to bring a lot of duplicate codes, and also to make the whole
> code more readable, this commit extracts common codes into two new helpers
> parse_static_mem_prop and acquire_static_memory_bank.
>
> Signed-off-by: Penny Zheng <penny.zheng@arm.com>
> ---
> v3 changes:
> - new commit to move the split off of the code outside in a separate patch
> ---
> v4 changes:
> - use mfn_eq() instead, because it is the only value used to indicate
> there is an error and this is more lightweight than mfn_valid()
> ---
> xen/arch/arm/domain_build.c | 100 +++++++++++++++++++++++-------------
> 1 file changed, 64 insertions(+), 36 deletions(-)
>
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index 5a106a977c..9206ec908d 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -509,12 +509,69 @@ static bool __init append_static_memory_to_bank(struct domain *d,
> return true;
> }
>
> +static mfn_t __init acquire_static_memory_bank(struct domain *d,
> + const __be32 **cell,
> + u32 addr_cells, u32 size_cells,
> + paddr_t *pbase, paddr_t *psize)
NIT: we usually align the parameters:
static mfn_t __init acquire_static_memory_bank(struct domain *d,
const __be32 **cell,
u32 addr_cells, u32 size_cells,
paddr_t *pbase, paddr_t *psize)
with that addressed:
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
> +{
> + mfn_t smfn;
> + int res;
> +
> + device_tree_get_reg(cell, addr_cells, size_cells, pbase, psize);
> + ASSERT(IS_ALIGNED(*pbase, PAGE_SIZE) && IS_ALIGNED(*psize, PAGE_SIZE));
> + if ( PFN_DOWN(*psize) > UINT_MAX )
> + {
> + printk(XENLOG_ERR "%pd: static memory size too large: %#"PRIpaddr,
> + d, *psize);
> + return INVALID_MFN;
> + }
> +
> + smfn = maddr_to_mfn(*pbase);
> + res = acquire_domstatic_pages(d, smfn, PFN_DOWN(*psize), 0);
> + if ( res )
> + {
> + printk(XENLOG_ERR
> + "%pd: failed to acquire static memory: %d.\n", d, res);
> + return INVALID_MFN;
> + }
> +
> + return smfn;
> +}
> +
> +static int __init parse_static_mem_prop(const struct dt_device_node *node,
> + u32 *addr_cells, u32 *size_cells,
> + int *length, const __be32 **cell)
> +{
> + const struct dt_property *prop;
> +
> + prop = dt_find_property(node, "xen,static-mem", NULL);
> + if ( !dt_property_read_u32(node, "#xen,static-mem-address-cells",
> + addr_cells) )
> + {
> + printk(XENLOG_ERR
> + "failed to read \"#xen,static-mem-address-cells\".\n");
> + return -EINVAL;
> + }
> +
> + if ( !dt_property_read_u32(node, "#xen,static-mem-size-cells",
> + size_cells) )
> + {
> + printk(XENLOG_ERR
> + "failed to read \"#xen,static-mem-size-cells\".\n");
> + return -EINVAL;
> + }
> +
> + *cell = (const __be32 *)prop->value;
> + *length = prop->length;
> +
> + return 0;
> +}
> +
> /* Allocate memory from static memory as RAM for one specific domain d. */
> static void __init allocate_static_memory(struct domain *d,
> struct kernel_info *kinfo,
> const struct dt_device_node *node)
> {
> - const struct dt_property *prop;
> u32 addr_cells, size_cells, reg_cells;
> unsigned int nr_banks, gbank, bank = 0;
> const uint64_t rambase[] = GUEST_RAM_BANK_BASES;
> @@ -523,24 +580,10 @@ static void __init allocate_static_memory(struct domain *d,
> u64 tot_size = 0;
> paddr_t pbase, psize, gsize;
> mfn_t smfn;
> - int res;
> -
> - prop = dt_find_property(node, "xen,static-mem", NULL);
> - if ( !dt_property_read_u32(node, "#xen,static-mem-address-cells",
> - &addr_cells) )
> - {
> - printk(XENLOG_ERR
> - "%pd: failed to read \"#xen,static-mem-address-cells\".\n", d);
> - goto fail;
> - }
> + int length;
>
> - if ( !dt_property_read_u32(node, "#xen,static-mem-size-cells",
> - &size_cells) )
> - {
> - printk(XENLOG_ERR
> - "%pd: failed to read \"#xen,static-mem-size-cells\".\n", d);
> + if ( parse_static_mem_prop(node, &addr_cells, &size_cells, &length, &cell) )
> goto fail;
> - }
> reg_cells = addr_cells + size_cells;
>
> /*
> @@ -551,29 +594,14 @@ static void __init allocate_static_memory(struct domain *d,
> gbank = 0;
> gsize = ramsize[gbank];
> kinfo->mem.bank[gbank].start = rambase[gbank];
> -
> - cell = (const __be32 *)prop->value;
> - nr_banks = (prop->length) / (reg_cells * sizeof (u32));
> + nr_banks = length / (reg_cells * sizeof (u32));
>
> for ( ; bank < nr_banks; bank++ )
> {
> - device_tree_get_reg(&cell, addr_cells, size_cells, &pbase, &psize);
> - ASSERT(IS_ALIGNED(pbase, PAGE_SIZE) && IS_ALIGNED(psize, PAGE_SIZE));
> -
> - if ( PFN_DOWN(psize) > UINT_MAX )
> - {
> - printk(XENLOG_ERR "%pd: static memory size too large: %#"PRIpaddr,
> - d, psize);
> + smfn = acquire_static_memory_bank(d, &cell, addr_cells, size_cells,
> + &pbase, &psize);
> + if ( mfn_eq(smfn, INVALID_MFN) )
> goto fail;
> - }
> - smfn = maddr_to_mfn(pbase);
> - res = acquire_domstatic_pages(d, smfn, PFN_DOWN(psize), 0);
> - if ( res )
> - {
> - printk(XENLOG_ERR
> - "%pd: failed to acquire static memory: %d.\n", d, res);
> - goto fail;
> - }
>
> printk(XENLOG_INFO "%pd: STATIC BANK[%u] %#"PRIpaddr"-%#"PRIpaddr"\n",
> d, bank, pbase, pbase + psize);
> --
> 2.25.1
>
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v4 05/11] xen/arm: introduce direct-map for domUs
2021-12-20 5:21 [PATCH v4 00/11] direct-map memory map Penny Zheng
` (3 preceding siblings ...)
2021-12-20 5:21 ` [PATCH v4 04/11] xen/arm: introduce new helper parse_static_mem_prop and acquire_static_memory_bank Penny Zheng
@ 2021-12-20 5:21 ` Penny Zheng
2022-01-13 22:53 ` Stefano Stabellini
2021-12-20 5:21 ` [PATCH v4 06/11] xen/arm: add ASSERT_UNREACHABLE in allocate_static_memory Penny Zheng
` (6 subsequent siblings)
11 siblings, 1 reply; 20+ messages in thread
From: Penny Zheng @ 2021-12-20 5:21 UTC (permalink / raw)
To: xen-devel, sstabellini, julien; +Cc: Bertrand.Marquis, Wei.Chen
Cases where domU needs direct-map memory map:
* IOMMU not present in the system.
* IOMMU disabled if it doesn't cover a specific device and all the guests
are trusted. Thinking a mixed scenario, where a few devices with IOMMU and
a few without, then guest DMA security still could not be totally guaranteed.
So users may want to disable the IOMMU, to at least gain some performance
improvement from IOMMU disabled.
* IOMMU disabled as a workaround when it doesn't have enough bandwidth.
To be specific, in a few extreme situation, when multiple devices do DMA
concurrently, these requests may exceed IOMMU's transmission capacity.
* IOMMU disabled when it adds too much latency on DMA. For example,
TLB may be missing in some IOMMU hardware, which may bring latency in DMA
progress, so users may want to disable it in some realtime scenario.
* Guest OS relies on the host memory layout
This commit introduces a new helper assign_static_memory_11 to allocate
static memory as guest RAM for direct-map domain.
Signed-off-by: Penny Zheng <penny.zheng@arm.com>
---
v2 changes:
- split the common codes into two helpers: parse_static_mem_prop and
acquire_static_memory_bank to deduce complexity.
- introduce a new helper allocate_static_memory_11 for allocating static
memory for direct-map guests
- remove redundant use "bool direct_map", to be replaced by
d_cfg.flags & XEN_DOMCTL_CDF_directmap
- remove panic action since it is fine to assign a non-DMA capable device when
IOMMU and direct-map both off
---
v3 changes:
- doc refinement
- drop the pointless gbank
- add check of the size of nr_banks shall not exceed NR_MEM_BANKS
- add ASSERT_UNREACHABLE to catch any misuse
- add another check of validating flag XEN_DOMCTL_CDF_INTERNAL_directmap only
when CONFIG_STATIC_MEMORY is set.
---
v4 changes:
- comment refinement
- rename function allocate_static_memory_11() to assign_static_memory_11()
to make clear there is actually no allocation done. Instead we are only
mapping pre-defined host regions to pre-defined guest regions.
- remove tot_size to directly substract psize from kinfo->unassigned_mem
- check kinfo->unassigned_mem doesn't underflow or overflow
- remove nested if/else
- refine "panic" info
---
xen/arch/arm/domain_build.c | 97 +++++++++++++++++++++++++++++++++++--
1 file changed, 94 insertions(+), 3 deletions(-)
diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 9206ec908d..d74a3eb908 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -494,8 +494,17 @@ static bool __init append_static_memory_to_bank(struct domain *d,
{
int res;
unsigned int nr_pages = PFN_DOWN(size);
- /* Infer next GFN. */
- gfn_t sgfn = gaddr_to_gfn(bank->start + bank->size);
+ gfn_t sgfn;
+
+ /*
+ * For direct-mapped domain, the GFN match the MFN.
+ * Otherwise, this is inferred on what has already been allocated
+ * in the bank.
+ */
+ if ( !is_domain_direct_mapped(d) )
+ sgfn = gaddr_to_gfn(bank->start + bank->size);
+ else
+ sgfn = gaddr_to_gfn(mfn_to_maddr(smfn));
res = guest_physmap_add_pages(d, sgfn, smfn, nr_pages);
if ( res )
@@ -668,12 +677,92 @@ static void __init allocate_static_memory(struct domain *d,
fail:
panic("Failed to allocate requested static memory for domain %pd.", d);
}
+
+/*
+ * Allocate static memory as RAM for one specific domain d.
+ * The static memory will be directly mapped in the guest(Guest Physical
+ * Address == Physical Address).
+ */
+static void __init assign_static_memory_11(struct domain *d,
+ struct kernel_info *kinfo,
+ const struct dt_device_node *node)
+{
+ u32 addr_cells, size_cells, reg_cells;
+ unsigned int nr_banks, bank = 0;
+ const __be32 *cell;
+ paddr_t pbase, psize;
+ mfn_t smfn;
+ int length;
+
+ if ( parse_static_mem_prop(node, &addr_cells, &size_cells, &length, &cell) )
+ {
+ printk(XENLOG_ERR
+ "%pd: failed to parse \"xen,static-mem\" property.\n", d);
+ goto fail;
+ }
+ reg_cells = addr_cells + size_cells;
+ nr_banks = length / (reg_cells * sizeof (u32));
+
+ if ( nr_banks > NR_MEM_BANKS )
+ {
+ printk(XENLOG_ERR
+ "%pd: exceed max number of supported guest memory banks.\n", d);
+ goto fail;
+ }
+
+ for ( ; bank < nr_banks; bank++ )
+ {
+ smfn = acquire_static_memory_bank(d, &cell, addr_cells, size_cells,
+ &pbase, &psize);
+ if ( mfn_eq(smfn, INVALID_MFN) )
+ goto fail;
+
+ printk(XENLOG_INFO "%pd: STATIC BANK[%u] %#"PRIpaddr"-%#"PRIpaddr"\n",
+ d, bank, pbase, pbase + psize);
+
+ /* One guest memory bank is matched with one physical memory bank. */
+ kinfo->mem.bank[bank].start = pbase;
+ if ( !append_static_memory_to_bank(d, &kinfo->mem.bank[bank],
+ smfn, psize) )
+ goto fail;
+
+ kinfo->unassigned_mem -= psize;
+ }
+
+ kinfo->mem.nr_banks = nr_banks;
+
+ /*
+ * The property 'memory' should match the amount of memory given to
+ * the guest.
+ * Currently, it is only possible to either acquire static memory or
+ * let Xen allocate. *Mixing* is not supported.
+ */
+ if ( kinfo->unassigned_mem != 0 )
+ {
+ printk(XENLOG_ERR
+ "Size of \"memory\" property doesn't match up with the sum-up of \"xen,static-mem\". Unsupported configuration.\n");
+ goto fail;
+ }
+
+ return;
+
+ fail:
+ panic("Failed to assign requested static memory for direct-map domain %pd.",
+ d);
+}
#else
static void __init allocate_static_memory(struct domain *d,
struct kernel_info *kinfo,
const struct dt_device_node *node)
{
}
+
+static void __init assign_static_memory_11(struct domain *d,
+ struct kernel_info *kinfo,
+ const struct dt_device_node *node)
+{
+ ASSERT_UNREACHABLE();
+}
#endif
/*
@@ -3023,8 +3112,10 @@ static int __init construct_domU(struct domain *d,
#endif
if ( !dt_find_property(node, "xen,static-mem", NULL) )
allocate_memory(d, &kinfo);
- else
+ else if ( !is_domain_direct_mapped(d) )
allocate_static_memory(d, &kinfo, node);
+ else
+ assign_static_memory_11(d, &kinfo, node);
rc = prepare_dtb_domU(d, &kinfo);
if ( rc < 0 )
--
2.25.1
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH v4 05/11] xen/arm: introduce direct-map for domUs
2021-12-20 5:21 ` [PATCH v4 05/11] xen/arm: introduce direct-map for domUs Penny Zheng
@ 2022-01-13 22:53 ` Stefano Stabellini
2022-01-24 17:48 ` Julien Grall
0 siblings, 1 reply; 20+ messages in thread
From: Stefano Stabellini @ 2022-01-13 22:53 UTC (permalink / raw)
To: Penny Zheng; +Cc: xen-devel, sstabellini, julien, Bertrand.Marquis, Wei.Chen
On Mon, 20 Dec 2021, Penny Zheng wrote:
> Cases where domU needs direct-map memory map:
> * IOMMU not present in the system.
> * IOMMU disabled if it doesn't cover a specific device and all the guests
> are trusted. Thinking a mixed scenario, where a few devices with IOMMU and
> a few without, then guest DMA security still could not be totally guaranteed.
> So users may want to disable the IOMMU, to at least gain some performance
> improvement from IOMMU disabled.
> * IOMMU disabled as a workaround when it doesn't have enough bandwidth.
> To be specific, in a few extreme situation, when multiple devices do DMA
> concurrently, these requests may exceed IOMMU's transmission capacity.
> * IOMMU disabled when it adds too much latency on DMA. For example,
> TLB may be missing in some IOMMU hardware, which may bring latency in DMA
> progress, so users may want to disable it in some realtime scenario.
> * Guest OS relies on the host memory layout
>
> This commit introduces a new helper assign_static_memory_11 to allocate
> static memory as guest RAM for direct-map domain.
>
> Signed-off-by: Penny Zheng <penny.zheng@arm.com>
The patch looks good. There are a couple of minor code style issus below
that it would be good to fix, at least the function parameters
alignment. With that:
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
> ---
> v2 changes:
> - split the common codes into two helpers: parse_static_mem_prop and
> acquire_static_memory_bank to deduce complexity.
> - introduce a new helper allocate_static_memory_11 for allocating static
> memory for direct-map guests
> - remove redundant use "bool direct_map", to be replaced by
> d_cfg.flags & XEN_DOMCTL_CDF_directmap
> - remove panic action since it is fine to assign a non-DMA capable device when
> IOMMU and direct-map both off
> ---
> v3 changes:
> - doc refinement
> - drop the pointless gbank
> - add check of the size of nr_banks shall not exceed NR_MEM_BANKS
> - add ASSERT_UNREACHABLE to catch any misuse
> - add another check of validating flag XEN_DOMCTL_CDF_INTERNAL_directmap only
> when CONFIG_STATIC_MEMORY is set.
> ---
> v4 changes:
> - comment refinement
> - rename function allocate_static_memory_11() to assign_static_memory_11()
> to make clear there is actually no allocation done. Instead we are only
> mapping pre-defined host regions to pre-defined guest regions.
> - remove tot_size to directly substract psize from kinfo->unassigned_mem
> - check kinfo->unassigned_mem doesn't underflow or overflow
> - remove nested if/else
> - refine "panic" info
> ---
> xen/arch/arm/domain_build.c | 97 +++++++++++++++++++++++++++++++++++--
> 1 file changed, 94 insertions(+), 3 deletions(-)
>
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index 9206ec908d..d74a3eb908 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -494,8 +494,17 @@ static bool __init append_static_memory_to_bank(struct domain *d,
> {
> int res;
> unsigned int nr_pages = PFN_DOWN(size);
> - /* Infer next GFN. */
> - gfn_t sgfn = gaddr_to_gfn(bank->start + bank->size);
> + gfn_t sgfn;
> +
> + /*
> + * For direct-mapped domain, the GFN match the MFN.
> + * Otherwise, this is inferred on what has already been allocated
> + * in the bank.
> + */
> + if ( !is_domain_direct_mapped(d) )
> + sgfn = gaddr_to_gfn(bank->start + bank->size);
> + else
> + sgfn = gaddr_to_gfn(mfn_to_maddr(smfn));
>
> res = guest_physmap_add_pages(d, sgfn, smfn, nr_pages);
> if ( res )
> @@ -668,12 +677,92 @@ static void __init allocate_static_memory(struct domain *d,
> fail:
> panic("Failed to allocate requested static memory for domain %pd.", d);
> }
> +
> +/*
> + * Allocate static memory as RAM for one specific domain d.
> + * The static memory will be directly mapped in the guest(Guest Physical
> + * Address == Physical Address).
> + */
> +static void __init assign_static_memory_11(struct domain *d,
> + struct kernel_info *kinfo,
> + const struct dt_device_node *node)
Please align the parameters
> +{
> + u32 addr_cells, size_cells, reg_cells;
> + unsigned int nr_banks, bank = 0;
> + const __be32 *cell;
> + paddr_t pbase, psize;
> + mfn_t smfn;
> + int length;
> +
> + if ( parse_static_mem_prop(node, &addr_cells, &size_cells, &length, &cell) )
> + {
> + printk(XENLOG_ERR
> + "%pd: failed to parse \"xen,static-mem\" property.\n", d);
> + goto fail;
> + }
> + reg_cells = addr_cells + size_cells;
> + nr_banks = length / (reg_cells * sizeof (u32));
no space after sizeof
> +
> + if ( nr_banks > NR_MEM_BANKS )
> + {
> + printk(XENLOG_ERR
> + "%pd: exceed max number of supported guest memory banks.\n", d);
> + goto fail;
> + }
> +
> + for ( ; bank < nr_banks; bank++ )
> + {
> + smfn = acquire_static_memory_bank(d, &cell, addr_cells, size_cells,
> + &pbase, &psize);
> + if ( mfn_eq(smfn, INVALID_MFN) )
> + goto fail;
> +
> + printk(XENLOG_INFO "%pd: STATIC BANK[%u] %#"PRIpaddr"-%#"PRIpaddr"\n",
> + d, bank, pbase, pbase + psize);
> +
> + /* One guest memory bank is matched with one physical memory bank. */
> + kinfo->mem.bank[bank].start = pbase;
> + if ( !append_static_memory_to_bank(d, &kinfo->mem.bank[bank],
> + smfn, psize) )
> + goto fail;
> +
> + kinfo->unassigned_mem -= psize;
> + }
> +
> + kinfo->mem.nr_banks = nr_banks;
> +
> + /*
> + * The property 'memory' should match the amount of memory given to
> + * the guest.
> + * Currently, it is only possible to either acquire static memory or
> + * let Xen allocate. *Mixing* is not supported.
> + */
> + if ( kinfo->unassigned_mem != 0 )
> + {
> + printk(XENLOG_ERR
> + "Size of \"memory\" property doesn't match up with the sum-up of \"xen,static-mem\". Unsupported configuration.\n");
This line would benefit from being broken down, but I am also OK if we
leave it as is
> + goto fail;
> + }
> +
> + return;
> +
> + fail:
> + panic("Failed to assign requested static memory for direct-map domain %pd.",
> + d);
> +}
> #else
> static void __init allocate_static_memory(struct domain *d,
> struct kernel_info *kinfo,
> const struct dt_device_node *node)
> {
> }
> +
> +static void __init assign_static_memory_11(struct domain *d,
> + struct kernel_info *kinfo,
> + const struct dt_device_node *node)
> +{
> + ASSERT_UNREACHABLE();
> +}
> #endif
>
> /*
> @@ -3023,8 +3112,10 @@ static int __init construct_domU(struct domain *d,
> #endif
> if ( !dt_find_property(node, "xen,static-mem", NULL) )
> allocate_memory(d, &kinfo);
> - else
> + else if ( !is_domain_direct_mapped(d) )
> allocate_static_memory(d, &kinfo, node);
> + else
> + assign_static_memory_11(d, &kinfo, node);
>
> rc = prepare_dtb_domU(d, &kinfo);
> if ( rc < 0 )
> --
> 2.25.1
>
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH v4 05/11] xen/arm: introduce direct-map for domUs
2022-01-13 22:53 ` Stefano Stabellini
@ 2022-01-24 17:48 ` Julien Grall
0 siblings, 0 replies; 20+ messages in thread
From: Julien Grall @ 2022-01-24 17:48 UTC (permalink / raw)
To: Stefano Stabellini, Penny Zheng; +Cc: xen-devel, Bertrand.Marquis, Wei.Chen
Hi,
On 13/01/2022 22:53, Stefano Stabellini wrote:
>> + kinfo->mem.nr_banks = nr_banks;
>> +
>> + /*
>> + * The property 'memory' should match the amount of memory given to
>> + * the guest.
>> + * Currently, it is only possible to either acquire static memory or
>> + * let Xen allocate. *Mixing* is not supported.
>> + */
>> + if ( kinfo->unassigned_mem != 0 )
>> + {
>> + printk(XENLOG_ERR
>> + "Size of \"memory\" property doesn't match up with the sum-up of \"xen,static-mem\". Unsupported configuration.\n");
>
> This line would benefit from being broken down, but I am also OK if we
> leave it as is
We usually keep the message in a single line because (even if it is more
than 80 characters) because it helps to find the line afterwards.
Looking at the message, I would drop "Unsupported configuration" because
it implies that this is because some code is missing (IOW it will be
supported in the future). However, this is a requirement.
Cheers,
--
Julien Grall
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v4 06/11] xen/arm: add ASSERT_UNREACHABLE in allocate_static_memory
2021-12-20 5:21 [PATCH v4 00/11] direct-map memory map Penny Zheng
` (4 preceding siblings ...)
2021-12-20 5:21 ` [PATCH v4 05/11] xen/arm: introduce direct-map for domUs Penny Zheng
@ 2021-12-20 5:21 ` Penny Zheng
2021-12-20 5:21 ` [PATCH v4 07/11] xen/arm: if direct-map domain use native addresses for GICv2 Penny Zheng
` (5 subsequent siblings)
11 siblings, 0 replies; 20+ messages in thread
From: Penny Zheng @ 2021-12-20 5:21 UTC (permalink / raw)
To: xen-devel, sstabellini, julien; +Cc: Bertrand.Marquis, Wei.Chen
Helper allocate_static_memory is not meant to be reachable when built with
!CONFIG_STATIC_MEMORY, so this commit adds ASSERT_UNREACHABLE in it to catch
potential misuse.
Signed-off-by: Penny Zheng <penny.zheng@arm.com>
Acked-by: Julien Grall <jgrall@amazon.com>
---
v3 changes:
- new commit
---
xen/arch/arm/domain_build.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index d74a3eb908..b706e674c9 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -755,6 +755,7 @@ static void __init allocate_static_memory(struct domain *d,
struct kernel_info *kinfo,
const struct dt_device_node *node)
{
+ ASSERT_UNREACHABLE();
}
static void __init assign_static_memory_11(struct domain *d,
--
2.25.1
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH v4 07/11] xen/arm: if direct-map domain use native addresses for GICv2
2021-12-20 5:21 [PATCH v4 00/11] direct-map memory map Penny Zheng
` (5 preceding siblings ...)
2021-12-20 5:21 ` [PATCH v4 06/11] xen/arm: add ASSERT_UNREACHABLE in allocate_static_memory Penny Zheng
@ 2021-12-20 5:21 ` Penny Zheng
2021-12-20 5:21 ` [PATCH v4 08/11] xen/arm: gate make_gicv3_domU_node with CONFIG_GICV3 Penny Zheng
` (4 subsequent siblings)
11 siblings, 0 replies; 20+ messages in thread
From: Penny Zheng @ 2021-12-20 5:21 UTC (permalink / raw)
To: xen-devel, sstabellini, julien; +Cc: Bertrand.Marquis, Wei.Chen
From: Stefano Stabellini <stefano.stabellini@xilinx.com>
Today we use native addresses to map the GICv2 for Dom0 and fixed
addresses for DomUs.
This patch changes the behavior so that native addresses are used for
all domains that are direct-mapped.
NEW VGIC has different naming schemes, like referring distributor base
address as vgic_dist_base, other than the dbase. So this patch also introduces
vgic_dist_base/vgic_cpu_base accessor to access correct distributor base
address/cpu interface base address on varied scenarios,
Signed-off-by: Stefano Stabellini <stefano.stabellini@xilinx.com>
Signed-off-by: Penny Zheng <penny.zheng@arm.com>
---
v2 changes
- combine all changes in patch 4-6 here
---
v3 changes
- refine comment message
- add a comment explaining how the 38 was found of "char buf[38]"
- simply map the CPU interface at the GPA vgic_v2_hw.cbase
- remove a spurious change
---
v4 changes:
- refine comment to let it be a summary of the if/else if/else.
---
xen/arch/arm/domain_build.c | 11 +++++++---
xen/arch/arm/include/asm/new_vgic.h | 10 +++++++++
xen/arch/arm/include/asm/vgic.h | 11 ++++++++++
xen/arch/arm/vgic-v2.c | 34 +++++++++++++++++++++++------
xen/arch/arm/vgic/vgic-v2.c | 34 +++++++++++++++++++++++------
5 files changed, 83 insertions(+), 17 deletions(-)
diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index b706e674c9..4788b03d8b 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -2273,8 +2273,13 @@ static int __init make_gicv2_domU_node(struct kernel_info *kinfo)
int res = 0;
__be32 reg[(GUEST_ROOT_ADDRESS_CELLS + GUEST_ROOT_SIZE_CELLS) * 2];
__be32 *cells;
+ const struct domain *d = kinfo->d;
+ /* Placeholder for interrupt-controller@ + a 64-bit number + \0 */
+ char buf[38];
- res = fdt_begin_node(fdt, "interrupt-controller@"__stringify(GUEST_GICD_BASE));
+ snprintf(buf, sizeof(buf), "interrupt-controller@%"PRIx64,
+ vgic_dist_base(&d->arch.vgic));
+ res = fdt_begin_node(fdt, buf);
if ( res )
return res;
@@ -2296,9 +2301,9 @@ static int __init make_gicv2_domU_node(struct kernel_info *kinfo)
cells = ®[0];
dt_child_set_range(&cells, GUEST_ROOT_ADDRESS_CELLS, GUEST_ROOT_SIZE_CELLS,
- GUEST_GICD_BASE, GUEST_GICD_SIZE);
+ vgic_dist_base(&d->arch.vgic), GUEST_GICD_SIZE);
dt_child_set_range(&cells, GUEST_ROOT_ADDRESS_CELLS, GUEST_ROOT_SIZE_CELLS,
- GUEST_GICC_BASE, GUEST_GICC_SIZE);
+ vgic_cpu_base(&d->arch.vgic), GUEST_GICC_SIZE);
res = fdt_property(fdt, "reg", reg, sizeof(reg));
if (res)
diff --git a/xen/arch/arm/include/asm/new_vgic.h b/xen/arch/arm/include/asm/new_vgic.h
index 97d622bff6..ab57fcd91d 100644
--- a/xen/arch/arm/include/asm/new_vgic.h
+++ b/xen/arch/arm/include/asm/new_vgic.h
@@ -186,6 +186,16 @@ struct vgic_cpu {
uint32_t num_id_bits;
};
+static inline paddr_t vgic_cpu_base(const struct vgic_dist *vgic)
+{
+ return vgic->vgic_cpu_base;
+}
+
+static inline paddr_t vgic_dist_base(const struct vgic_dist *vgic)
+{
+ return vgic->vgic_dist_base;
+}
+
#endif /* __ASM_ARM_NEW_VGIC_H */
/*
diff --git a/xen/arch/arm/include/asm/vgic.h b/xen/arch/arm/include/asm/vgic.h
index ade427a808..d2a9fc7d83 100644
--- a/xen/arch/arm/include/asm/vgic.h
+++ b/xen/arch/arm/include/asm/vgic.h
@@ -152,6 +152,7 @@ struct vgic_dist {
struct pending_irq *pending_irqs;
/* Base address for guest GIC */
paddr_t dbase; /* Distributor base address */
+ paddr_t cbase; /* CPU interface base address */
#ifdef CONFIG_GICV3
/* GIC V3 addressing */
/* List of contiguous occupied by the redistributors */
@@ -271,6 +272,16 @@ static inline int REG_RANK_NR(int b, uint32_t n)
enum gic_sgi_mode;
+static inline paddr_t vgic_cpu_base(const struct vgic_dist *vgic)
+{
+ return vgic->cbase;
+}
+
+static inline paddr_t vgic_dist_base(const struct vgic_dist *vgic)
+{
+ return vgic->dbase;
+}
+
/*
* Offset of GICD_<FOO><n> with its rank, for GICD_<FOO> size <s> with
* <b>-bits-per-interrupt.
diff --git a/xen/arch/arm/vgic-v2.c b/xen/arch/arm/vgic-v2.c
index 589c033eda..b1bd7a46ad 100644
--- a/xen/arch/arm/vgic-v2.c
+++ b/xen/arch/arm/vgic-v2.c
@@ -654,12 +654,16 @@ static int vgic_v2_vcpu_init(struct vcpu *v)
static int vgic_v2_domain_init(struct domain *d)
{
int ret;
- paddr_t cbase, csize;
+ paddr_t csize;
paddr_t vbase;
/*
- * The hardware domain gets the hardware address.
- * Guests get the virtual platform layout.
+ * The hardware domain and direct-mapped domain both get the hardware
+ * address.
+ * We have to handle them separately because the hwdom is re-using the
+ * same Device-Tree as the host (see more details below).
+ *
+ * Other domains get the virtual platform layout.
*/
if ( is_hardware_domain(d) )
{
@@ -671,10 +675,26 @@ static int vgic_v2_domain_init(struct domain *d)
* Note that we assume the size of the CPU interface is always
* aligned to PAGE_SIZE.
*/
- cbase = vgic_v2_hw.cbase;
+ d->arch.vgic.cbase = vgic_v2_hw.cbase;
csize = vgic_v2_hw.csize;
vbase = vgic_v2_hw.vbase;
}
+ else if ( is_domain_direct_mapped(d) )
+ {
+ /*
+ * For all the direct-mapped domain other than the hardware domain,
+ * we only map a 8kB CPU interface but we make sure it is at a
+ * location occupied by the physical GIC in the host device tree.
+ *
+ * We need to add an offset to the virtual CPU interface base
+ * address when the GIC is aliased to get a 8kB contiguous
+ * region.
+ */
+ d->arch.vgic.dbase = vgic_v2_hw.dbase;
+ d->arch.vgic.cbase = vgic_v2_hw.cbase;
+ csize = GUEST_GICC_SIZE;
+ vbase = vgic_v2_hw.vbase + vgic_v2_hw.aliased_offset;
+ }
else
{
d->arch.vgic.dbase = GUEST_GICD_BASE;
@@ -685,7 +705,7 @@ static int vgic_v2_domain_init(struct domain *d)
* region.
*/
BUILD_BUG_ON(GUEST_GICC_SIZE != SZ_8K);
- cbase = GUEST_GICC_BASE;
+ d->arch.vgic.cbase = GUEST_GICC_BASE;
csize = GUEST_GICC_SIZE;
vbase = vgic_v2_hw.vbase + vgic_v2_hw.aliased_offset;
}
@@ -694,8 +714,8 @@ static int vgic_v2_domain_init(struct domain *d)
* Map the gic virtual cpu interface in the gic cpu interface
* region of the guest.
*/
- ret = map_mmio_regions(d, gaddr_to_gfn(cbase), csize / PAGE_SIZE,
- maddr_to_mfn(vbase));
+ ret = map_mmio_regions(d, gaddr_to_gfn(d->arch.vgic.cbase),
+ csize / PAGE_SIZE, maddr_to_mfn(vbase));
if ( ret )
return ret;
diff --git a/xen/arch/arm/vgic/vgic-v2.c b/xen/arch/arm/vgic/vgic-v2.c
index b5ba4ace87..1a99d3a8b4 100644
--- a/xen/arch/arm/vgic/vgic-v2.c
+++ b/xen/arch/arm/vgic/vgic-v2.c
@@ -258,13 +258,17 @@ void vgic_v2_enable(struct vcpu *vcpu)
int vgic_v2_map_resources(struct domain *d)
{
struct vgic_dist *dist = &d->arch.vgic;
- paddr_t cbase, csize;
+ paddr_t csize;
paddr_t vbase;
int ret;
/*
- * The hardware domain gets the hardware address.
- * Guests get the virtual platform layout.
+ * The hardware domain and direct-mapped domain both get the hardware
+ * address.
+ * We have to handle them separately because the hwdom is re-using the
+ * same Device-Tree as the host (see more details below).
+ *
+ * Other domains get the virtual platform layout.
*/
if ( is_hardware_domain(d) )
{
@@ -276,10 +280,26 @@ int vgic_v2_map_resources(struct domain *d)
* Note that we assume the size of the CPU interface is always
* aligned to PAGE_SIZE.
*/
- cbase = gic_v2_hw_data.cbase;
+ d->arch.vgic.vgic_cpu_base = gic_v2_hw_data.cbase;
csize = gic_v2_hw_data.csize;
vbase = gic_v2_hw_data.vbase;
}
+ else if ( is_domain_direct_mapped(d) )
+ {
+ d->arch.vgic.vgic_dist_base = gic_v2_hw_data.dbase;
+ /*
+ * For all the direct-mapped domain other than the hardware domain,
+ * we only map a 8kB CPU interface but we make sure it is at a location
+ * occupied by the physical GIC in the host device tree.
+ *
+ * We need to add an offset to the virtual CPU interface base
+ * address when the GIC is aliased to get a 8kB contiguous
+ * region.
+ */
+ d->arch.vgic.vgic_cpu_base = gic_v2_hw_data.cbase;
+ csize = GUEST_GICC_SIZE;
+ vbase = gic_v2_hw_data.vbase + gic_v2_hw_data.aliased_offset;
+ }
else
{
d->arch.vgic.vgic_dist_base = GUEST_GICD_BASE;
@@ -290,7 +310,7 @@ int vgic_v2_map_resources(struct domain *d)
* region.
*/
BUILD_BUG_ON(GUEST_GICC_SIZE != SZ_8K);
- cbase = GUEST_GICC_BASE;
+ d->arch.vgic.vgic_cpu_base = GUEST_GICC_BASE;
csize = GUEST_GICC_SIZE;
vbase = gic_v2_hw_data.vbase + gic_v2_hw_data.aliased_offset;
}
@@ -308,8 +328,8 @@ int vgic_v2_map_resources(struct domain *d)
* Map the gic virtual cpu interface in the gic cpu interface
* region of the guest.
*/
- ret = map_mmio_regions(d, gaddr_to_gfn(cbase), csize / PAGE_SIZE,
- maddr_to_mfn(vbase));
+ ret = map_mmio_regions(d, gaddr_to_gfn(d->arch.vgic.vgic_cpu_base),
+ csize / PAGE_SIZE, maddr_to_mfn(vbase));
if ( ret )
{
gdprintk(XENLOG_ERR, "Unable to remap VGIC CPU to VCPU\n");
--
2.25.1
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH v4 08/11] xen/arm: gate make_gicv3_domU_node with CONFIG_GICV3
2021-12-20 5:21 [PATCH v4 00/11] direct-map memory map Penny Zheng
` (6 preceding siblings ...)
2021-12-20 5:21 ` [PATCH v4 07/11] xen/arm: if direct-map domain use native addresses for GICv2 Penny Zheng
@ 2021-12-20 5:21 ` Penny Zheng
2022-01-13 22:54 ` Stefano Stabellini
2021-12-20 5:21 ` [PATCH v4 09/11] xen/arm: if direct-map domain use native addresses for GICv3 Penny Zheng
` (3 subsequent siblings)
11 siblings, 1 reply; 20+ messages in thread
From: Penny Zheng @ 2021-12-20 5:21 UTC (permalink / raw)
To: xen-devel, sstabellini, julien; +Cc: Bertrand.Marquis, Wei.Chen
This commit gates function make_gicv3_domU_node with CONFIG_GICV3.
Signed-off-by: Penny Zheng <penny.zheng@arm.com>
---
v4 changes:
- remove ASSERT_UNREACHABLE() to avoid breaking compilation on prod build with
CONFIG_GICV3=n
---
xen/arch/arm/domain_build.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 4788b03d8b..139d428524 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -2322,6 +2322,7 @@ static int __init make_gicv2_domU_node(struct kernel_info *kinfo)
return res;
}
+#ifdef CONFIG_GICV3
static int __init make_gicv3_domU_node(struct kernel_info *kinfo)
{
void *fdt = kinfo->fdt;
@@ -2371,13 +2372,16 @@ static int __init make_gicv3_domU_node(struct kernel_info *kinfo)
return res;
}
+#endif
static int __init make_gic_domU_node(struct kernel_info *kinfo)
{
switch ( kinfo->d->arch.vgic.version )
{
+#ifdef CONFIG_GICV3
case GIC_V3:
return make_gicv3_domU_node(kinfo);
+#endif
case GIC_V2:
return make_gicv2_domU_node(kinfo);
default:
--
2.25.1
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH v4 08/11] xen/arm: gate make_gicv3_domU_node with CONFIG_GICV3
2021-12-20 5:21 ` [PATCH v4 08/11] xen/arm: gate make_gicv3_domU_node with CONFIG_GICV3 Penny Zheng
@ 2022-01-13 22:54 ` Stefano Stabellini
0 siblings, 0 replies; 20+ messages in thread
From: Stefano Stabellini @ 2022-01-13 22:54 UTC (permalink / raw)
To: Penny Zheng; +Cc: xen-devel, sstabellini, julien, Bertrand.Marquis, Wei.Chen
On Mon, 20 Dec 2021, Penny Zheng wrote:
> This commit gates function make_gicv3_domU_node with CONFIG_GICV3.
>
> Signed-off-by: Penny Zheng <penny.zheng@arm.com>
Acked-by: Stefano Stabellini <sstabellini@kernel.org>
> ---
> v4 changes:
> - remove ASSERT_UNREACHABLE() to avoid breaking compilation on prod build with
> CONFIG_GICV3=n
> ---
> xen/arch/arm/domain_build.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index 4788b03d8b..139d428524 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -2322,6 +2322,7 @@ static int __init make_gicv2_domU_node(struct kernel_info *kinfo)
> return res;
> }
>
> +#ifdef CONFIG_GICV3
> static int __init make_gicv3_domU_node(struct kernel_info *kinfo)
> {
> void *fdt = kinfo->fdt;
> @@ -2371,13 +2372,16 @@ static int __init make_gicv3_domU_node(struct kernel_info *kinfo)
>
> return res;
> }
> +#endif
>
> static int __init make_gic_domU_node(struct kernel_info *kinfo)
> {
> switch ( kinfo->d->arch.vgic.version )
> {
> +#ifdef CONFIG_GICV3
> case GIC_V3:
> return make_gicv3_domU_node(kinfo);
> +#endif
> case GIC_V2:
> return make_gicv2_domU_node(kinfo);
> default:
> --
> 2.25.1
>
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v4 09/11] xen/arm: if direct-map domain use native addresses for GICv3
2021-12-20 5:21 [PATCH v4 00/11] direct-map memory map Penny Zheng
` (7 preceding siblings ...)
2021-12-20 5:21 ` [PATCH v4 08/11] xen/arm: gate make_gicv3_domU_node with CONFIG_GICV3 Penny Zheng
@ 2021-12-20 5:21 ` Penny Zheng
2021-12-20 5:21 ` [PATCH v4 10/11] xen/arm: if direct-map domain use native UART address and IRQ number for vPL011 Penny Zheng
` (2 subsequent siblings)
11 siblings, 0 replies; 20+ messages in thread
From: Penny Zheng @ 2021-12-20 5:21 UTC (permalink / raw)
To: xen-devel, sstabellini, julien; +Cc: Bertrand.Marquis, Wei.Chen
From: Stefano Stabellini <stefano.stabellini@xilinx.com>
Today we use native addresses to map the GICv3 for Dom0 and fixed
addresses for DomUs.
This patch changes the behavior so that native addresses are used for
all domain which is using the host memory layout
Considering that DOM0 may not always be directly mapped in the future,
this patch introduces a new helper "domain_use_host_layout()" that
wraps both two check "is_domain_direct_mapped(d) || is_hardware_domain(d)"
for more flexible usage.
Signed-off-by: Stefano Stabellini <stefano.stabellini@xilinx.com>
Signed-off-by: Penny Zheng <penny.zheng@arm.com>
---
v2 changes:
- remove redistributor accessor
- introduce new helper "is_domain_use_host_layout()"
- comment fix
---
v3 changes:
- the comment on top of 'buf' to explain how 38 was found
- fix res getting overwritten
- drop 'cells += (GUEST_ROOT_ADDRESS_CELLS + GUEST_ROOT_SIZE_CELLS)'
- free 'reg' right way
- fix comment
- rename 'is_domain_use_host_layout()' to 'domain_use_host_layout()'
---
v4 changes:
- refine comment
---
xen/arch/arm/domain_build.c | 34 +++++++++++++++++++++++--------
xen/arch/arm/include/asm/domain.h | 14 +++++++++++++
xen/arch/arm/vgic-v3.c | 26 ++++++++++++-----------
3 files changed, 54 insertions(+), 20 deletions(-)
diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 139d428524..9a7145b3ee 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -2327,10 +2327,16 @@ static int __init make_gicv3_domU_node(struct kernel_info *kinfo)
{
void *fdt = kinfo->fdt;
int res = 0;
- __be32 reg[(GUEST_ROOT_ADDRESS_CELLS + GUEST_ROOT_SIZE_CELLS) * 2];
- __be32 *cells;
+ __be32 *reg, *cells;
+ const struct domain *d = kinfo->d;
+ /* Placeholder for interrupt-controller@ + a 64-bit number + \0 */
+ char buf[38];
+ unsigned int i, len = 0;
+
+ snprintf(buf, sizeof(buf), "interrupt-controller@%"PRIx64,
+ vgic_dist_base(&d->arch.vgic));
- res = fdt_begin_node(fdt, "interrupt-controller@"__stringify(GUEST_GICV3_GICD_BASE));
+ res = fdt_begin_node(fdt, buf);
if ( res )
return res;
@@ -2350,13 +2356,25 @@ static int __init make_gicv3_domU_node(struct kernel_info *kinfo)
if ( res )
return res;
- cells = ®[0];
- dt_child_set_range(&cells, GUEST_ROOT_ADDRESS_CELLS, GUEST_ROOT_SIZE_CELLS,
- GUEST_GICV3_GICD_BASE, GUEST_GICV3_GICD_SIZE);
+ /* reg specifies all re-distributors and Distributor. */
+ len = (GUEST_ROOT_ADDRESS_CELLS + GUEST_ROOT_SIZE_CELLS) *
+ (d->arch.vgic.nr_regions + 1) * sizeof(__be32);
+ reg = xmalloc_bytes(len);
+ if ( reg == NULL )
+ return -ENOMEM;
+ cells = reg;
+
dt_child_set_range(&cells, GUEST_ROOT_ADDRESS_CELLS, GUEST_ROOT_SIZE_CELLS,
- GUEST_GICV3_GICR0_BASE, GUEST_GICV3_GICR0_SIZE);
+ vgic_dist_base(&d->arch.vgic), GUEST_GICV3_GICD_SIZE);
- res = fdt_property(fdt, "reg", reg, sizeof(reg));
+ for ( i = 0; i < d->arch.vgic.nr_regions; i++)
+ dt_child_set_range(&cells,
+ GUEST_ROOT_ADDRESS_CELLS, GUEST_ROOT_SIZE_CELLS,
+ d->arch.vgic.rdist_regions[i].base,
+ d->arch.vgic.rdist_regions[i].size);
+
+ res = fdt_property(fdt, "reg", reg, len);
+ xfree(reg);
if (res)
return res;
diff --git a/xen/arch/arm/include/asm/domain.h b/xen/arch/arm/include/asm/domain.h
index cb37ce89ec..848fec8a0f 100644
--- a/xen/arch/arm/include/asm/domain.h
+++ b/xen/arch/arm/include/asm/domain.h
@@ -31,6 +31,20 @@ enum domain_type {
#define is_domain_direct_mapped(d) (d->arch.directmap)
+/*
+ * Is the domain using the host memory layout?
+ *
+ * Direct-mapped domain will always have the RAM mapped with GFN == MFN.
+ * To avoid any trouble finding space, it is easier to force using the
+ * host memory layout.
+ *
+ * The hardware domain will use the host layout regardless of
+ * direct-mapped because some OS may rely on a specific address ranges
+ * for the devices.
+ */
+#define domain_use_host_layout(d) (is_domain_direct_mapped(d) || \
+ is_hardware_domain(d))
+
struct vtimer {
struct vcpu *v;
int irq;
diff --git a/xen/arch/arm/vgic-v3.c b/xen/arch/arm/vgic-v3.c
index 65bb7991a6..144089a7b6 100644
--- a/xen/arch/arm/vgic-v3.c
+++ b/xen/arch/arm/vgic-v3.c
@@ -1640,14 +1640,15 @@ static inline unsigned int vgic_v3_max_rdist_count(struct domain *d)
* Normally there is only one GICv3 redistributor region.
* The GICv3 DT binding provisions for multiple regions, since there are
* platforms out there which need those (multi-socket systems).
- * For Dom0 we have to live with the MMIO layout the hardware provides,
- * so we have to copy the multiple regions - as the first region may not
- * provide enough space to hold all redistributors we need.
+ * For domain using the host memory layout, we have to live with the MMIO
+ * layout the hardware provides, so we have to copy the multiple regions
+ * - as the first region may not provide enough space to hold all
+ * redistributors we need.
* However DomU get a constructed memory map, so we can go with
* the architected single redistributor region.
*/
- return is_hardware_domain(d) ? vgic_v3_hw.nr_rdist_regions :
- GUEST_GICV3_RDIST_REGIONS;
+ return domain_use_host_layout(d) ? vgic_v3_hw.nr_rdist_regions :
+ GUEST_GICV3_RDIST_REGIONS;
}
static int vgic_v3_domain_init(struct domain *d)
@@ -1669,10 +1670,11 @@ static int vgic_v3_domain_init(struct domain *d)
radix_tree_init(&d->arch.vgic.pend_lpi_tree);
/*
- * Domain 0 gets the hardware address.
- * Guests get the virtual platform layout.
+ * For domain using the host memory layout, it gets the hardware
+ * address.
+ * Other domains get the virtual platform layout.
*/
- if ( is_hardware_domain(d) )
+ if ( domain_use_host_layout(d) )
{
unsigned int first_cpu = 0;
@@ -1695,10 +1697,10 @@ static int vgic_v3_domain_init(struct domain *d)
}
/*
- * The hardware domain may not use all the re-distributors
- * regions (e.g when the number of vCPUs does not match the
- * number of pCPUs). Update the number of regions to avoid
- * exposing unused region as they will not get emulated.
+ * For domain using the host memory layout, it may not use all
+ * the re-distributors regions (e.g when the number of vCPUs does
+ * not match the number of pCPUs). Update the number of regions to
+ * avoid exposing unused region as they will not get emulated.
*/
d->arch.vgic.nr_regions = i + 1;
--
2.25.1
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH v4 10/11] xen/arm: if direct-map domain use native UART address and IRQ number for vPL011
2021-12-20 5:21 [PATCH v4 00/11] direct-map memory map Penny Zheng
` (8 preceding siblings ...)
2021-12-20 5:21 ` [PATCH v4 09/11] xen/arm: if direct-map domain use native addresses for GICv3 Penny Zheng
@ 2021-12-20 5:21 ` Penny Zheng
2021-12-20 5:21 ` [PATCH v4 11/11] xen/docs: Document how to do passthrough without IOMMU Penny Zheng
2022-01-13 22:55 ` [PATCH v4 00/11] direct-map memory map Stefano Stabellini
11 siblings, 0 replies; 20+ messages in thread
From: Penny Zheng @ 2021-12-20 5:21 UTC (permalink / raw)
To: xen-devel, sstabellini, julien; +Cc: Bertrand.Marquis, Wei.Chen
From: Stefano Stabellini <stefano.stabellini@xilinx.com>
We always use a fix address to map the vPL011 to domains. The address
could be a problem for direct-map domains.
So, for domains that are directly mapped, reuse the address of the
physical UART on the platform to avoid potential clashes.
Do the same for the virtual IRQ number: instead of always using
GUEST_VPL011_SPI, try to reuse the physical SPI number if possible.
Signed-off-by: Stefano Stabellini <stefano.stabellini@xilinx.com>
Signed-off-by: Penny Zheng <penny.zheng@arm.com>
---
v2 changes:
- explain why vpl011 initialization before creating its device tree node
- error out if the domain is direct-mapped and the IRQ is not found
- harden the code and add a check/comment when the hardware UART region
is smaller than GUEST_VPL011_SIZE.
---
v3 changes:
- explain how the '27' was found for 'buf'
- fix checking before dereferencing
- refine comment message
---
v4 changes:
- refine comment message
---
xen/arch/arm/domain_build.c | 44 +++++++++++++++++++----
xen/arch/arm/include/asm/vpl011.h | 2 ++
xen/arch/arm/vpl011.c | 60 +++++++++++++++++++++++++++----
3 files changed, 92 insertions(+), 14 deletions(-)
diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 9a7145b3ee..a3196a4545 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -30,6 +30,7 @@
#include <xen/irq.h>
#include <xen/grant_table.h>
+#include <xen/serial.h>
static unsigned int __initdata opt_dom0_max_vcpus;
integer_param("dom0_max_vcpus", opt_dom0_max_vcpus);
@@ -2415,8 +2416,12 @@ static int __init make_vpl011_uart_node(struct kernel_info *kinfo)
gic_interrupt_t intr;
__be32 reg[GUEST_ROOT_ADDRESS_CELLS + GUEST_ROOT_SIZE_CELLS];
__be32 *cells;
+ struct domain *d = kinfo->d;
+ /* Placeholder for sbsa-uart@ + a 64-bit number + \0 */
+ char buf[27];
- res = fdt_begin_node(fdt, "sbsa-uart@"__stringify(GUEST_PL011_BASE));
+ snprintf(buf, sizeof(buf), "sbsa-uart@%"PRIx64, d->arch.vpl011.base_addr);
+ res = fdt_begin_node(fdt, buf);
if ( res )
return res;
@@ -2426,14 +2431,14 @@ static int __init make_vpl011_uart_node(struct kernel_info *kinfo)
cells = ®[0];
dt_child_set_range(&cells, GUEST_ROOT_ADDRESS_CELLS,
- GUEST_ROOT_SIZE_CELLS, GUEST_PL011_BASE,
+ GUEST_ROOT_SIZE_CELLS, d->arch.vpl011.base_addr,
GUEST_PL011_SIZE);
res = fdt_property(fdt, "reg", reg, sizeof(reg));
if ( res )
return res;
- set_interrupt(intr, GUEST_VPL011_SPI, 0xf, DT_IRQ_TYPE_LEVEL_HIGH);
+ set_interrupt(intr, d->arch.vpl011.virq, 0xf, DT_IRQ_TYPE_LEVEL_HIGH);
res = fdt_property(fdt, "interrupts", intr, sizeof (intr));
if ( res )
@@ -3145,6 +3150,14 @@ static int __init construct_domU(struct domain *d,
else
assign_static_memory_11(d, &kinfo, node);
+ /*
+ * Base address and irq number are needed when creating vpl011 device
+ * tree node in prepare_dtb_domU, so initialization on related variables
+ * shall be done first.
+ */
+ if ( kinfo.vpl011 )
+ rc = domain_vpl011_init(d, NULL);
+
rc = prepare_dtb_domU(d, &kinfo);
if ( rc < 0 )
return rc;
@@ -3153,9 +3166,6 @@ static int __init construct_domU(struct domain *d,
if ( rc < 0 )
return rc;
- if ( kinfo.vpl011 )
- rc = domain_vpl011_init(d, NULL);
-
return rc;
}
@@ -3200,15 +3210,35 @@ void __init create_domUs(void)
if ( !dt_property_read_u32(node, "nr_spis", &d_cfg.arch.nr_spis) )
{
+ unsigned int vpl011_virq = GUEST_VPL011_SPI;
+
d_cfg.arch.nr_spis = gic_number_lines() - 32;
+ /*
+ * The VPL011 virq is GUEST_VPL011_SPI, unless direct-map is
+ * set, in which case it'll match the hardware.
+ *
+ * Since the domain is not yet created, we can't use
+ * d->arch.vpl011.irq. So the logic to find the vIRQ has to
+ * be hardcoded.
+ * The logic here shall be consistent with the one in
+ * domain_vpl011_init().
+ */
+ if ( flags & CDF_directmap )
+ {
+ vpl011_virq = serial_irq(SERHND_DTUART);
+ if ( vpl011_virq < 0 )
+ panic("Error getting IRQ number for this serial port %d\n",
+ SERHND_DTUART);
+ }
+
/*
* vpl011 uses one emulated SPI. If vpl011 is requested, make
* sure that we allocate enough SPIs for it.
*/
if ( dt_property_read_bool(node, "vpl011") )
d_cfg.arch.nr_spis = MAX(d_cfg.arch.nr_spis,
- GUEST_VPL011_SPI - 32 + 1);
+ vpl011_virq - 32 + 1);
}
/*
diff --git a/xen/arch/arm/include/asm/vpl011.h b/xen/arch/arm/include/asm/vpl011.h
index e6c7ab7381..c09abcd7a9 100644
--- a/xen/arch/arm/include/asm/vpl011.h
+++ b/xen/arch/arm/include/asm/vpl011.h
@@ -53,6 +53,8 @@ struct vpl011 {
uint32_t uarticr; /* Interrupt clear register */
uint32_t uartris; /* Raw interrupt status register */
uint32_t shadow_uartmis; /* shadow masked interrupt register */
+ paddr_t base_addr;
+ unsigned int virq;
spinlock_t lock;
evtchn_port_t evtchn;
};
diff --git a/xen/arch/arm/vpl011.c b/xen/arch/arm/vpl011.c
index 895f436cc4..43522d48fd 100644
--- a/xen/arch/arm/vpl011.c
+++ b/xen/arch/arm/vpl011.c
@@ -29,6 +29,7 @@
#include <xen/mm.h>
#include <xen/sched.h>
#include <xen/console.h>
+#include <xen/serial.h>
#include <public/domctl.h>
#include <public/io/console.h>
#include <asm/pl011-uart.h>
@@ -71,11 +72,11 @@ static void vpl011_update_interrupt_status(struct domain *d)
* status bit has been set since the last time.
*/
if ( uartmis & ~vpl011->shadow_uartmis )
- vgic_inject_irq(d, NULL, GUEST_VPL011_SPI, true);
+ vgic_inject_irq(d, NULL, vpl011->virq, true);
vpl011->shadow_uartmis = uartmis;
#else
- vgic_inject_irq(d, NULL, GUEST_VPL011_SPI, uartmis);
+ vgic_inject_irq(d, NULL, vpl011->virq, uartmis);
#endif
}
@@ -347,7 +348,8 @@ static int vpl011_mmio_read(struct vcpu *v,
void *priv)
{
struct hsr_dabt dabt = info->dabt;
- uint32_t vpl011_reg = (uint32_t)(info->gpa - GUEST_PL011_BASE);
+ uint32_t vpl011_reg = (uint32_t)(info->gpa -
+ v->domain->arch.vpl011.base_addr);
struct vpl011 *vpl011 = &v->domain->arch.vpl011;
struct domain *d = v->domain;
unsigned long flags;
@@ -430,7 +432,8 @@ static int vpl011_mmio_write(struct vcpu *v,
void *priv)
{
struct hsr_dabt dabt = info->dabt;
- uint32_t vpl011_reg = (uint32_t)(info->gpa - GUEST_PL011_BASE);
+ uint32_t vpl011_reg = (uint32_t)(info->gpa -
+ v->domain->arch.vpl011.base_addr);
struct vpl011 *vpl011 = &v->domain->arch.vpl011;
struct domain *d = v->domain;
unsigned long flags;
@@ -626,6 +629,49 @@ int domain_vpl011_init(struct domain *d, struct vpl011_init_info *info)
if ( vpl011->backend.dom.ring_buf )
return -EINVAL;
+ /*
+ * The VPL011 virq is GUEST_VPL011_SPI, except for direct-map domains
+ * where the hardware value shall be used.
+ * The logic here should stay in sync with the one in
+ * create_domUs().
+ */
+ if ( is_domain_direct_mapped(d) )
+ {
+ const struct vuart_info *uart = serial_vuart_info(SERHND_DTUART);
+ int vpl011_irq = serial_irq(SERHND_DTUART);
+
+ if ( (uart != NULL) && (vpl011_irq > 0) )
+ {
+ vpl011->base_addr = uart->base_addr;
+ vpl011->virq = vpl011_irq;
+ }
+ else
+ {
+ printk(XENLOG_ERR
+ "vpl011: Unable to re-use the Xen UART information.\n");
+ return -EINVAL;
+ }
+
+ /*
+ * Since the PL011 we emulate for the guest requires a 4KB region,
+ * and on some Hardware (e.g. on some sunxi SoC), the UART MMIO
+ * region is less than 4KB, in which case, there may exist multiple
+ * devices within the same 4KB region, here adds the following check to
+ * prevent potential known pitfalls
+ */
+ if ( uart->size < GUEST_PL011_SIZE )
+ {
+ printk(XENLOG_ERR
+ "vpl011: Can't re-use the Xen UART MMIO region as it is too small.\n");
+ return -EINVAL;
+ }
+ }
+ else
+ {
+ vpl011->base_addr = GUEST_PL011_BASE;
+ vpl011->virq = GUEST_VPL011_SPI;
+ }
+
/*
* info is NULL when the backend is in Xen.
* info is != NULL when the backend is in a domain.
@@ -661,7 +707,7 @@ int domain_vpl011_init(struct domain *d, struct vpl011_init_info *info)
}
}
- rc = vgic_reserve_virq(d, GUEST_VPL011_SPI);
+ rc = vgic_reserve_virq(d, vpl011->virq);
if ( !rc )
{
rc = -EINVAL;
@@ -673,12 +719,12 @@ int domain_vpl011_init(struct domain *d, struct vpl011_init_info *info)
spin_lock_init(&vpl011->lock);
register_mmio_handler(d, &vpl011_mmio_handler,
- GUEST_PL011_BASE, GUEST_PL011_SIZE, NULL);
+ vpl011->base_addr, GUEST_PL011_SIZE, NULL);
return 0;
out2:
- vgic_free_virq(d, GUEST_VPL011_SPI);
+ vgic_free_virq(d, vpl011->virq);
out1:
if ( vpl011->backend_in_domain )
--
2.25.1
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH v4 11/11] xen/docs: Document how to do passthrough without IOMMU
2021-12-20 5:21 [PATCH v4 00/11] direct-map memory map Penny Zheng
` (9 preceding siblings ...)
2021-12-20 5:21 ` [PATCH v4 10/11] xen/arm: if direct-map domain use native UART address and IRQ number for vPL011 Penny Zheng
@ 2021-12-20 5:21 ` Penny Zheng
2022-01-13 22:55 ` [PATCH v4 00/11] direct-map memory map Stefano Stabellini
11 siblings, 0 replies; 20+ messages in thread
From: Penny Zheng @ 2021-12-20 5:21 UTC (permalink / raw)
To: xen-devel, sstabellini, julien; +Cc: Bertrand.Marquis, Wei.Chen
From: Stefano Stabellini <stefano.stabellini@xilinx.com>
This commit creates a new doc to document how to do passthrough without IOMMU.
Signed-off-by: Stefano Stabellini <stefano.stabellini@xilinx.com>
Signed-off-by: Penny Zheng <penny.zheng@arm.com>
---
docs/misc/arm/passthrough-noiommu.txt | 52 +++++++++++++++++++++++++++
1 file changed, 52 insertions(+)
create mode 100644 docs/misc/arm/passthrough-noiommu.txt
diff --git a/docs/misc/arm/passthrough-noiommu.txt b/docs/misc/arm/passthrough-noiommu.txt
new file mode 100644
index 0000000000..3e2ef21ad7
--- /dev/null
+++ b/docs/misc/arm/passthrough-noiommu.txt
@@ -0,0 +1,52 @@
+Request Device Assignment without IOMMU support
+===============================================
+
+*WARNING:
+Users should be aware that it is not always secure to assign a device without
+IOMMU protection.
+When the device is not protected by the IOMMU, the administrator should make
+sure that:
+ 1. The device is assigned to a trusted guest.
+ 2. Users have additional security mechanism on the platform.
+
+This document assumes that the IOMMU is absent from the system or it is
+disabled (status = "disabled" in device tree).
+
+Add xen,force-assign-without-iommu; to the device tree snippet:
+
+ethernet: ethernet@ff0e0000 {
+ compatible = "cdns,zynqmp-gem";
+ xen,path = "/amba/ethernet@ff0e0000";
+ xen,reg = <0x0 0xff0e0000 0x1000 0x0 0xff0e0000>;
+ xen,force-assign-without-iommu;
+};
+
+Request 1:1 memory mapping for the domain on static allocation
+==============================================================
+
+Add a direct-map property under the appropriate /chosen/domU node which
+is also statically allocated with physical memory ranges through
+xen,static-mem property as its guest RAM.
+
+Below is an example on how to specify the 1:1 memory mapping for the domain
+on static allocation in the device-tree:
+
+/ {
+ chosen {
+ domU1 {
+ compatible = "xen,domain";
+ #address-cells = <0x2>;
+ #size-cells = <0x2>;
+ cpus = <2>;
+ memory = <0x0 0x80000>;
+ #xen,static-mem-address-cells = <0x1>;
+ #xen,static-mem-size-cells = <0x1>;
+ xen,static-mem = <0x30000000 0x20000000>;
+ direct-map;
+ ...
+ };
+ };
+};
+
+Besides reserving a 512MB region starting at the host physical address
+0x30000000 to DomU1, it also requests 1:1 memory mapping.
--
2.25.1
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH v4 00/11] direct-map memory map
2021-12-20 5:21 [PATCH v4 00/11] direct-map memory map Penny Zheng
` (10 preceding siblings ...)
2021-12-20 5:21 ` [PATCH v4 11/11] xen/docs: Document how to do passthrough without IOMMU Penny Zheng
@ 2022-01-13 22:55 ` Stefano Stabellini
11 siblings, 0 replies; 20+ messages in thread
From: Stefano Stabellini @ 2022-01-13 22:55 UTC (permalink / raw)
To: Penny Zheng; +Cc: xen-devel, sstabellini, julien, Bertrand.Marquis, Wei.Chen
Hi Penny,
Thanks for the update. I tested the series in a couple of different
configurations and it works great!
You can add my Tested-by to all patches
On Mon, 20 Dec 2021, Penny Zheng wrote:
> Cases where domU needs direct-map memory map:
> * IOMMU not present in the system.
> * IOMMU disabled if it doesn't cover a specific device and all the guests
> are trusted. Thinking a mixed scenario, where a few devices with IOMMU and
> a few without, then guest DMA security still could not be totally guaranteed.
> So users may want to disable the IOMMU, to at least gain some performance
> improvement from IOMMU disabled.
> * IOMMU disabled as a workaround when it doesn't have enough bandwidth.
> To be specific, in a few extreme situation, when multiple devices do DMA
> concurrently, these requests may exceed IOMMU's transmission capacity.
> * IOMMU disabled when it adds too much latency on DMA. For example,
> TLB may be missing in some IOMMU hardware, which may bring latency in DMA
> progress, so users may want to disable it in some realtime scenario.
> * Guest OS relies on the host memory layout
>
> "direct-map" property shall be added under the appropriate domain node,
> when users requesting direct-map memory mapping for the domain.
>
> Right now, direct-map is only supported when domain on Static Allocation,
> that is, "xen,static-mem" is also necessary in the domain configuration.
>
> Looking into related [design link](
> https://lists.xenproject.org/archives/html/xen-devel/2021-05/msg00882.html)
> for more details.
>
> The whole design is about Static Allocation and direct-map, and this
> Patch Serie only covers parts of it, which are direct-map memory map.
> Other features will be delievered through different patch series.
>
> See https://lists.xenproject.org/archives/html/xen-devel/2021-09/msg00855.html
> for Domain on Static Allocation.
>
> This patch serie is based on
> https://lists.xenproject.org/archives/html/xen-devel/2021-10/msg00822.html\
> ---
> v4 changes:
> - introduce internal const CDF_xxx flags for domain creation
> - introduce internal flag CDF_privileged
> - introduce new internal flag CDF_directmap
> - add a directmap flag under struct arch_domain and use it to
> reimplement is_domain_direct_mapped.
> - expand arch_domain_create/domain_create to include internal-only parameter
> "const unsigned int flags"
> - use mfn_eq() instead, because it is the only value used to indicate
> there is an error and this is more lightweight than mfn_valid()
> - rename function allocate_static_memory_11() to assign_static_memory_11()
> to make clear there is actually no allocation done. Instead we are only
> mapping pre-defined host regions to pre-defined guest regions.
> - remove tot_size to directly substract psize from kinfo->unassigned_mem
> - check kinfo->unassigned_mem doesn't underflow or overflow
> - remove nested if/else
> - remove ASSERT_UNREACHABLE() to avoid breaking compilation on prod build with
> CONFIG_GICV3=n
> - comment and commit message refinement
> ---
> v3 changes:
> - move flag XEN_DOMCTL_CDF_INTERNAL_directmap back to xen/include/xen/domain.h,
> to let it be only available for domain created by XEN.
> - name it with extra "INTERNAL" and add comments to warn developers not
> to accidently use its bitfield when introducing new XEN_DOMCTL_CDF_xxx flag.
> - reject this flag in x86'es arch_sanitise_domain_config()
> - add ASSERT_UNREACHABLE to catch any misuse in allocate_static_memory()
> and allocate_static_memory_11()
> - add another check of validating flag XEN_DOMCTL_CDF_INTERNAL_directmap only
> when CONFIG_STATIC_MEMORY is set.
> - simply map the CPU interface at the GPA vgic_v2_hw.cbase
> - drop 'cells += (GUEST_ROOT_ADDRESS_CELLS + GUEST_ROOT_SIZE_CELLS)'
> - rename 'is_domain_use_host_layout()' to 'domain_use_host_layout()'
> ---
> v2 changes:
> - remove the introduce of internal flag
> - Refine is_domain_direct_mapped to check whether the flag
> XEN_DOMCTL_CDF_directmap is set
> - reword "1:1 direct-map" to just "direct-map"
> - split the common codes into two helpers: parse_static_mem_prop and
> acquire_static_memory_bank to deduce complexity.
> - introduce a new helper allocate_static_memory_11 for allocating static
> memory for direct-map guests
> - remove panic action since it is fine to assign a non-DMA capable device when
> IOMMU and direct-map both off
> - remove redistributor accessor
> - introduce new helper "is_domain_use_host_layout()"
> - explain why vpl011 initialization before creating its device tree node
> - error out if the domain is direct-mapped and the IRQ is not found
> - harden the code and add a check/comment when the hardware UART region
> is smaller than CUEST_VPL011_SIZE.
> Penny Zheng (4):
> xen/arm: introduce new helper parse_static_mem_prop and
> acquire_static_memory_bank
> xen/arm: introduce direct-map for domUs
> xen/arm: add ASSERT_UNREACHABLE in allocate_static_memory
> xen/arm: gate make_gicv3_domU_node with CONFIG_GICV3
>
> Stefano Stabellini (7):
> xen: introduce internal CDF_xxx flags for domain creation
> xen: introduce CDF_directmap
> xen/arm: avoid setting XEN_DOMCTL_CDF_iommu when IOMMU off
> xen/arm: if direct-map domain use native addresses for GICv2
> xen/arm: if direct-map domain use native addresses for GICv3
> xen/arm: if direct-map domain use native UART address and IRQ number
> for vPL011
> xen/docs: Document how to do passthrough without IOMMU
>
> docs/misc/arm/device-tree/booting.txt | 6 +
> docs/misc/arm/passthrough-noiommu.txt | 52 +++++
> xen/arch/arm/domain.c | 5 +-
> xen/arch/arm/domain_build.c | 308 +++++++++++++++++++++-----
> xen/arch/arm/include/asm/domain.h | 19 +-
> xen/arch/arm/include/asm/new_vgic.h | 10 +
> xen/arch/arm/include/asm/vgic.h | 11 +
> xen/arch/arm/include/asm/vpl011.h | 2 +
> xen/arch/arm/vgic-v2.c | 34 ++-
> xen/arch/arm/vgic-v3.c | 26 ++-
> xen/arch/arm/vgic/vgic-v2.c | 34 ++-
> xen/arch/arm/vpl011.c | 60 ++++-
> xen/arch/x86/domain.c | 3 +-
> xen/arch/x86/setup.c | 2 +-
> xen/common/domain.c | 12 +-
> xen/common/sched/core.c | 2 +-
> xen/include/xen/domain.h | 9 +-
> xen/include/xen/sched.h | 2 +-
> 18 files changed, 490 insertions(+), 107 deletions(-)
> create mode 100644 docs/misc/arm/passthrough-noiommu.txt
>
> --
> 2.25.1
>
^ permalink raw reply [flat|nested] 20+ messages in thread