* [PATCH v6 01/11] xen: introduce internal CDF_xxx flags for domain creation
2022-02-14 3:19 [PATCH v6 00/11] direct-map memory map Penny Zheng
@ 2022-02-14 3:19 ` Penny Zheng
2022-02-14 3:19 ` [PATCH v6 02/11] xen: introduce CDF_directmap Penny Zheng
` (9 subsequent siblings)
10 siblings, 0 replies; 17+ messages in thread
From: Penny Zheng @ 2022-02-14 3:19 UTC (permalink / raw)
To: xen-devel, sstabellini, julien; +Cc: Bertrand.Marquis, Wei.Chen
From: Stefano Stabellini <sstabellini@kernel.org>
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 <sstabellini@kernel.org>
Signed-off-by: Penny Zheng <penny.zheng@arm.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
Tested-by: Stefano Stabellini <sstabellini@kernel.org>
---
v4 changes:
- new commit
---
v5 changes
- remove const constraint
---
v6 changes
- no changes
---
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 115f8f6517..624b53ded4 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..a79103e04a 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)
+ 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..24a9a87f83 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);
+ 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] 17+ messages in thread* [PATCH v6 02/11] xen: introduce CDF_directmap
2022-02-14 3:19 [PATCH v6 00/11] direct-map memory map Penny Zheng
2022-02-14 3:19 ` [PATCH v6 01/11] xen: introduce internal CDF_xxx flags for domain creation Penny Zheng
@ 2022-02-14 3:19 ` Penny Zheng
2022-02-15 20:26 ` Julien Grall
2022-02-14 3:19 ` [PATCH v6 03/11] xen/arm: Allow device-passthrough even the IOMMU is off Penny Zheng
` (8 subsequent siblings)
10 siblings, 1 reply; 17+ messages in thread
From: Penny Zheng @ 2022-02-14 3:19 UTC (permalink / raw)
To: xen-devel, sstabellini, julien; +Cc: Bertrand.Marquis, Wei.Chen
From: Stefano Stabellini <sstabellini@kernel.org>
This commit introduces a new arm-specific flag CDF_directmap to specify
that a domain should have its memory direct-map(guest physical address
== host 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 <sstabellini@kernel.org>
Signed-off-by: Penny Zheng <penny.zheng@arm.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
Tested-by: Stefano Stabellini <sstabellini@kernel.org>
---
CC: andrew.cooper3@citrix.com
CC: jbeulich@suse.com
CC: George Dunlap <George.Dunlap@eu.citrix.com>
CC: Ian Jackson <ian.jackson@eu.citrix.com>
CC: Wei Liu <wl@xen.org>
CC: "Roger Pau Monné" <roger.pau@citrix.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"
---
v5 changes
- remove const constraint
---
v6 changes
- comment and coding style fix
- protect CDF_directmap with #ifdef CONFIG_ARM
---
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 | 7 ++++++-
7 files changed, 34 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..8110c1df86 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,
+ 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..6467e8ee32 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) || !dt_find_property(node, "xen,static-mem", NULL) )
+ panic("direct-map is not valid for domain %s without static allocation.\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..aabe942cde 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..9835f90ea0 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,
+ unsigned int flags)
{
bool paging_initialised = false;
uint32_t emflags;
diff --git a/xen/common/domain.c b/xen/common/domain.c
index a79103e04a..3742322d22 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..24eb4cc7d3 100644
--- a/xen/include/xen/domain.h
+++ b/xen/include/xen/domain.h
@@ -31,6 +31,10 @@ 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)
+#ifdef CONFIG_ARM
+/* Should domain memory be directly mapped? */
+#define CDF_directmap (1U << 1)
+#endif
/*
* Arch-specifics.
@@ -65,7 +69,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,
+ unsigned int flags);
void arch_domain_destroy(struct domain *d);
--
2.25.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH v6 02/11] xen: introduce CDF_directmap
2022-02-14 3:19 ` [PATCH v6 02/11] xen: introduce CDF_directmap Penny Zheng
@ 2022-02-15 20:26 ` Julien Grall
2022-02-16 9:34 ` Jan Beulich
0 siblings, 1 reply; 17+ messages in thread
From: Julien Grall @ 2022-02-15 20:26 UTC (permalink / raw)
To: Penny Zheng, xen-devel, sstabellini
Cc: Bertrand.Marquis, Wei.Chen, Jan Beulich
(+ Jan)
Hi Penny,
I am CCing Jan to give him a chance to...
On 14/02/2022 03:19, Penny Zheng wrote:
> diff --git a/xen/include/xen/domain.h b/xen/include/xen/domain.h
> index cfb0b47f13..24eb4cc7d3 100644
> --- a/xen/include/xen/domain.h
> +++ b/xen/include/xen/domain.h
> @@ -31,6 +31,10 @@ 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)
> +#ifdef CONFIG_ARM
> +/* Should domain memory be directly mapped? */
> +#define CDF_directmap (1U << 1)
> +#endif
... comment on this approach. I would be happy to switch to an ASSERT()
if that's preferred.
Please note that as you modify x86 code (even it is a couple of lines)
you should technically CC the x86 maintainers. Similarly the changes in
include/xen/domain.h should have the REST CCed.
We have a script that will find the proper correct CC for each patch
(see scripts/add_maintainers.pl). The workflow is written down in the
script itself.
Cheers,
--
Julien Grall
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v6 02/11] xen: introduce CDF_directmap
2022-02-15 20:26 ` Julien Grall
@ 2022-02-16 9:34 ` Jan Beulich
2022-02-17 11:01 ` Julien Grall
0 siblings, 1 reply; 17+ messages in thread
From: Jan Beulich @ 2022-02-16 9:34 UTC (permalink / raw)
To: Julien Grall, Penny Zheng
Cc: Bertrand.Marquis, Wei.Chen, xen-devel, sstabellini
On 15.02.2022 21:26, Julien Grall wrote:
> (+ Jan)
>
> Hi Penny,
>
> I am CCing Jan to give him a chance to...
Thanks, but ...
> On 14/02/2022 03:19, Penny Zheng wrote:
>> diff --git a/xen/include/xen/domain.h b/xen/include/xen/domain.h
>> index cfb0b47f13..24eb4cc7d3 100644
>> --- a/xen/include/xen/domain.h
>> +++ b/xen/include/xen/domain.h
>> @@ -31,6 +31,10 @@ 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)
>> +#ifdef CONFIG_ARM
>> +/* Should domain memory be directly mapped? */
>> +#define CDF_directmap (1U << 1)
>> +#endif
>
> ... comment on this approach. I would be happy to switch to an ASSERT()
> if that's preferred.
... I think I did signal agreement with this approach beforehand.
It leaves the option to use the same bit for something x86-specific
down the road.
Jan
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v6 02/11] xen: introduce CDF_directmap
2022-02-16 9:34 ` Jan Beulich
@ 2022-02-17 11:01 ` Julien Grall
2022-02-17 19:47 ` Julien Grall
0 siblings, 1 reply; 17+ messages in thread
From: Julien Grall @ 2022-02-17 11:01 UTC (permalink / raw)
To: Jan Beulich, Penny Zheng
Cc: Bertrand.Marquis, Wei.Chen, xen-devel, sstabellini
Hi Jan,
On 16/02/2022 09:34, Jan Beulich wrote:
> On 15.02.2022 21:26, Julien Grall wrote:
>> (+ Jan)
>>
>> Hi Penny,
>>
>> I am CCing Jan to give him a chance to...
>
> Thanks, but ...
>
>> On 14/02/2022 03:19, Penny Zheng wrote:
>>> diff --git a/xen/include/xen/domain.h b/xen/include/xen/domain.h
>>> index cfb0b47f13..24eb4cc7d3 100644
>>> --- a/xen/include/xen/domain.h
>>> +++ b/xen/include/xen/domain.h
>>> @@ -31,6 +31,10 @@ 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)
>>> +#ifdef CONFIG_ARM
>>> +/* Should domain memory be directly mapped? */
>>> +#define CDF_directmap (1U << 1)
>>> +#endif
>>
>> ... comment on this approach. I would be happy to switch to an ASSERT()
>> if that's preferred.
>
> ... I think I did signal agreement with this approach beforehand.
You raised a concern and it wasn't 100% obvious whether you would still
be happy if we merged it as-is.
So I preferred to ask rather than merging it and getting an angry
e-mail/message afterwards :).
Anyway, this series is now fully acked. So I will commit the series in a
bit.
Cheers,
--
Julien Grall
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v6 02/11] xen: introduce CDF_directmap
2022-02-17 11:01 ` Julien Grall
@ 2022-02-17 19:47 ` Julien Grall
0 siblings, 0 replies; 17+ messages in thread
From: Julien Grall @ 2022-02-17 19:47 UTC (permalink / raw)
To: Penny Zheng; +Cc: Bertrand.Marquis, Wei.Chen, xen-devel, sstabellini
Hi,
On 17/02/2022 11:01, Julien Grall wrote:
> On 16/02/2022 09:34, Jan Beulich wrote:
>> On 15.02.2022 21:26, Julien Grall wrote:
>>> (+ Jan)
>>>
>>> Hi Penny,
>>>
>>> I am CCing Jan to give him a chance to...
>>
>> Thanks, but ...
>>
>>> On 14/02/2022 03:19, Penny Zheng wrote:
>>>> diff --git a/xen/include/xen/domain.h b/xen/include/xen/domain.h
>>>> index cfb0b47f13..24eb4cc7d3 100644
>>>> --- a/xen/include/xen/domain.h
>>>> +++ b/xen/include/xen/domain.h
>>>> @@ -31,6 +31,10 @@ 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)
>>>> +#ifdef CONFIG_ARM
>>>> +/* Should domain memory be directly mapped? */
>>>> +#define CDF_directmap (1U << 1)
>>>> +#endif
>>>
>>> ... comment on this approach. I would be happy to switch to an ASSERT()
>>> if that's preferred.
>>
>> ... I think I did signal agreement with this approach beforehand.
>
> You raised a concern and it wasn't 100% obvious whether you would still
> be happy if we merged it as-is.
>
> So I preferred to ask rather than merging it and getting an angry
> e-mail/message afterwards :).
>
> Anyway, this series is now fully acked. So I will commit the series in a
> bit.
Well, I forgot to explicitely Acked this patch. So:
Acked-by: Julien Grall <jgrall@amazon.com>
This series is now fully committed. Thank you for your contribution!
Cheers,
--
Julien Grall
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v6 03/11] xen/arm: Allow device-passthrough even the IOMMU is off
2022-02-14 3:19 [PATCH v6 00/11] direct-map memory map Penny Zheng
2022-02-14 3:19 ` [PATCH v6 01/11] xen: introduce internal CDF_xxx flags for domain creation Penny Zheng
2022-02-14 3:19 ` [PATCH v6 02/11] xen: introduce CDF_directmap Penny Zheng
@ 2022-02-14 3:19 ` Penny Zheng
2022-02-15 20:26 ` Julien Grall
2022-02-14 3:19 ` [PATCH v6 04/11] xen/arm: introduce new helper parse_static_mem_prop and acquire_static_memory_bank Penny Zheng
` (7 subsequent siblings)
10 siblings, 1 reply; 17+ messages in thread
From: Penny Zheng @ 2022-02-14 3:19 UTC (permalink / raw)
To: xen-devel, sstabellini, julien; +Cc: Bertrand.Marquis, Wei.Chen
From: Stefano Stabellini <sstabellini@kernel.org>
At the moment, we are only supporting device-passthrough when Xen has
enabled the IOMMU. There are some use cases where it is not possible to
use the IOMMU (e.g. doesn't exist, hardware limitation, performance) yet
it would be OK to assign a device to trusted domain so long they are
direct-mapped or the device doesn't do DMA.
Note that when the IOMMU is disabled, it will be necessary to add
xen,force-assign-without-iommu for every device that needs to be assigned.
Signed-off-by: Stefano Stabellini <stefano.stabellini@xilinx.com>
Signed-off-by: Penny Zheng <penny.zheng@arm.com>
Tested-by: Stefano Stabellini <sstabellini@kernel.org>
---
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.
---
v5 changes:
- nothing changed
---
v6 changes
- commit message refinement
---
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 6467e8ee32..c1e8c99f64 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] 17+ messages in thread* Re: [PATCH v6 03/11] xen/arm: Allow device-passthrough even the IOMMU is off
2022-02-14 3:19 ` [PATCH v6 03/11] xen/arm: Allow device-passthrough even the IOMMU is off Penny Zheng
@ 2022-02-15 20:26 ` Julien Grall
0 siblings, 0 replies; 17+ messages in thread
From: Julien Grall @ 2022-02-15 20:26 UTC (permalink / raw)
To: Penny Zheng, xen-devel, sstabellini; +Cc: Bertrand.Marquis, Wei.Chen
Hi,
On 14/02/2022 03:19, Penny Zheng wrote:
> From: Stefano Stabellini <sstabellini@kernel.org>
>
> At the moment, we are only supporting device-passthrough when Xen has
> enabled the IOMMU. There are some use cases where it is not possible to
> use the IOMMU (e.g. doesn't exist, hardware limitation, performance) yet
> it would be OK to assign a device to trusted domain so long they are
> direct-mapped or the device doesn't do DMA.
>
> Note that when the IOMMU is disabled, it will be necessary to add
> xen,force-assign-without-iommu for every device that needs to be assigned.
>
> Signed-off-by: Stefano Stabellini <stefano.stabellini@xilinx.com>
> Signed-off-by: Penny Zheng <penny.zheng@arm.com>
> Tested-by: Stefano Stabellini <sstabellini@kernel.org>
Acked-by: Julien Grall <jgrall@amazon.com>
Cheers,
--
Julien Grall
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v6 04/11] xen/arm: introduce new helper parse_static_mem_prop and acquire_static_memory_bank
2022-02-14 3:19 [PATCH v6 00/11] direct-map memory map Penny Zheng
` (2 preceding siblings ...)
2022-02-14 3:19 ` [PATCH v6 03/11] xen/arm: Allow device-passthrough even the IOMMU is off Penny Zheng
@ 2022-02-14 3:19 ` Penny Zheng
2022-02-14 3:19 ` [PATCH v6 05/11] xen/arm: introduce direct-map for domUs Penny Zheng
` (6 subsequent siblings)
10 siblings, 0 replies; 17+ messages in thread
From: Penny Zheng @ 2022-02-14 3:19 UTC (permalink / raw)
To: xen-devel, sstabellini, julien; +Cc: Bertrand.Marquis, Wei.Chen
Later, we will introduce assign_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>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
Tested-by: Stefano Stabellini <sstabellini@kernel.org>
---
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.
---
v5 changes
- fix coding style
---
v6 changes
- no changes
---
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 c1e8c99f64..e61d2d53ba 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] 17+ messages in thread* [PATCH v6 05/11] xen/arm: introduce direct-map for domUs
2022-02-14 3:19 [PATCH v6 00/11] direct-map memory map Penny Zheng
` (3 preceding siblings ...)
2022-02-14 3:19 ` [PATCH v6 04/11] xen/arm: introduce new helper parse_static_mem_prop and acquire_static_memory_bank Penny Zheng
@ 2022-02-14 3:19 ` Penny Zheng
2022-02-14 3:19 ` [PATCH v6 06/11] xen/arm: add ASSERT_UNREACHABLE in allocate_static_memory Penny Zheng
` (5 subsequent siblings)
10 siblings, 0 replies; 17+ messages in thread
From: Penny Zheng @ 2022-02-14 3:19 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>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
Tested-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
---
v5 changes:
- fix coding style
---
v6 changes:
- no changes
---
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 e61d2d53ba..ec29bd302c 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\".\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] 17+ messages in thread* [PATCH v6 06/11] xen/arm: add ASSERT_UNREACHABLE in allocate_static_memory
2022-02-14 3:19 [PATCH v6 00/11] direct-map memory map Penny Zheng
` (4 preceding siblings ...)
2022-02-14 3:19 ` [PATCH v6 05/11] xen/arm: introduce direct-map for domUs Penny Zheng
@ 2022-02-14 3:19 ` Penny Zheng
2022-02-14 3:19 ` [PATCH v6 07/11] xen/arm: if direct-map domain use native addresses for GICv2 Penny Zheng
` (4 subsequent siblings)
10 siblings, 0 replies; 17+ messages in thread
From: Penny Zheng @ 2022-02-14 3:19 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>
Tested-by: Stefano Stabellini <sstabellini@kernel.org>
---
v3 changes:
- new commit
---
v4 changes:
- nothing changed
---
v5 changes:
- nothing changed
---
v6 changes:
- nothing changed
---
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 ec29bd302c..52f256de9c 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] 17+ messages in thread* [PATCH v6 07/11] xen/arm: if direct-map domain use native addresses for GICv2
2022-02-14 3:19 [PATCH v6 00/11] direct-map memory map Penny Zheng
` (5 preceding siblings ...)
2022-02-14 3:19 ` [PATCH v6 06/11] xen/arm: add ASSERT_UNREACHABLE in allocate_static_memory Penny Zheng
@ 2022-02-14 3:19 ` Penny Zheng
2022-02-14 3:19 ` [PATCH v6 08/11] xen/arm: gate make_gicv3_domU_node with CONFIG_GICV3 Penny Zheng
` (3 subsequent siblings)
10 siblings, 0 replies; 17+ messages in thread
From: Penny Zheng @ 2022-02-14 3:19 UTC (permalink / raw)
To: xen-devel, sstabellini, julien; +Cc: Bertrand.Marquis, Wei.Chen
From: Stefano Stabellini <sstabellini@kernel.org>
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 <sstabellini@kernel.org>
Signed-off-by: Penny Zheng <penny.zheng@arm.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
Tested-by: Stefano Stabellini <sstabellini@kernel.org>
---
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.
---
v5 changes:
- nothing changed
---
v6 changes:
- nothing changed
---
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 52f256de9c..87391adde6 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] 17+ messages in thread* [PATCH v6 08/11] xen/arm: gate make_gicv3_domU_node with CONFIG_GICV3
2022-02-14 3:19 [PATCH v6 00/11] direct-map memory map Penny Zheng
` (6 preceding siblings ...)
2022-02-14 3:19 ` [PATCH v6 07/11] xen/arm: if direct-map domain use native addresses for GICv2 Penny Zheng
@ 2022-02-14 3:19 ` Penny Zheng
2022-02-14 3:19 ` [PATCH v6 09/11] xen/arm: if direct-map domain use native addresses for GICv3 Penny Zheng
` (2 subsequent siblings)
10 siblings, 0 replies; 17+ messages in thread
From: Penny Zheng @ 2022-02-14 3:19 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>
Acked-by: Stefano Stabellini <sstabellini@kernel.org>
Tested-by: Stefano Stabellini <sstabellini@kernel.org>
---
v4 changes:
- remove ASSERT_UNREACHABLE() to avoid breaking compilation on prod build with
CONFIG_GICV3=n
---
v5 changes:
- nothing changed
---
v6 changes:
- nothing changed
---
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 87391adde6..a01dc60b55 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] 17+ messages in thread* [PATCH v6 09/11] xen/arm: if direct-map domain use native addresses for GICv3
2022-02-14 3:19 [PATCH v6 00/11] direct-map memory map Penny Zheng
` (7 preceding siblings ...)
2022-02-14 3:19 ` [PATCH v6 08/11] xen/arm: gate make_gicv3_domU_node with CONFIG_GICV3 Penny Zheng
@ 2022-02-14 3:19 ` Penny Zheng
2022-02-14 3:19 ` [PATCH v6 10/11] xen/arm: if direct-map domain use native UART address and IRQ number for vPL011 Penny Zheng
2022-02-14 3:19 ` [PATCH v6 11/11] xen/docs: Document how to do passthrough without IOMMU Penny Zheng
10 siblings, 0 replies; 17+ messages in thread
From: Penny Zheng @ 2022-02-14 3:19 UTC (permalink / raw)
To: xen-devel, sstabellini, julien; +Cc: Bertrand.Marquis, Wei.Chen
From: Stefano Stabellini <sstabellini@kernel.org>
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 <sstabellini@kernel.org>
Signed-off-by: Penny Zheng <penny.zheng@arm.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
Tested-by: Stefano Stabellini <sstabellini@kernel.org>
---
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
---
v5 changes:
- nothing changed
---
v6 changes:
- refine comment
---
xen/arch/arm/domain_build.c | 34 +++++++++++++++++++++++--------
xen/arch/arm/include/asm/domain.h | 14 +++++++++++++
xen/arch/arm/vgic-v3.c | 30 ++++++++++++++-------------
3 files changed, 56 insertions(+), 22 deletions(-)
diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index a01dc60b55..cff2cb93cc 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 aabe942cde..c56f6e4398 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..e4ba9a6476 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.
- * However DomU get a constructed memory map, so we can go with
- * the architected single redistributor region.
+ * 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.
+ * All the other domains will 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] 17+ messages in thread* [PATCH v6 10/11] xen/arm: if direct-map domain use native UART address and IRQ number for vPL011
2022-02-14 3:19 [PATCH v6 00/11] direct-map memory map Penny Zheng
` (8 preceding siblings ...)
2022-02-14 3:19 ` [PATCH v6 09/11] xen/arm: if direct-map domain use native addresses for GICv3 Penny Zheng
@ 2022-02-14 3:19 ` Penny Zheng
2022-02-14 3:19 ` [PATCH v6 11/11] xen/docs: Document how to do passthrough without IOMMU Penny Zheng
10 siblings, 0 replies; 17+ messages in thread
From: Penny Zheng @ 2022-02-14 3:19 UTC (permalink / raw)
To: xen-devel, sstabellini, julien; +Cc: Bertrand.Marquis, Wei.Chen
From: Stefano Stabellini <sstabellini@kernel.org>
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 <sstabellini@kernel.org>
Signed-off-by: Penny Zheng <penny.zheng@arm.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
Tested-by: Stefano Stabellini <sstabellini@kernel.org>
---
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
---
v5 changes:
- nothing changed
---
v6 changes:
- nothing changed
---
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 cff2cb93cc..8be01678de 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] 17+ messages in thread* [PATCH v6 11/11] xen/docs: Document how to do passthrough without IOMMU
2022-02-14 3:19 [PATCH v6 00/11] direct-map memory map Penny Zheng
` (9 preceding siblings ...)
2022-02-14 3:19 ` [PATCH v6 10/11] xen/arm: if direct-map domain use native UART address and IRQ number for vPL011 Penny Zheng
@ 2022-02-14 3:19 ` Penny Zheng
10 siblings, 0 replies; 17+ messages in thread
From: Penny Zheng @ 2022-02-14 3:19 UTC (permalink / raw)
To: xen-devel, sstabellini, julien; +Cc: Bertrand.Marquis, Wei.Chen
From: Stefano Stabellini <sstabellini@kernel.org>
This commit creates a new doc to document how to do passthrough without IOMMU.
Signed-off-by: Stefano Stabellini <sstabellini@kernel.org>
Signed-off-by: Penny Zheng <penny.zheng@arm.com>
Acked-by: Julien Grall <jgrall@amazon.com>
Tested-by: Stefano Stabellini <sstabellini@kernel.org>
---
v2 changes:
- nothing changed
---
v3 changes:
- nothing changed
---
v4 changes:
- nothing changed
---
v5 changes:
- nothing changed
---
v6 changes:
- nothing changed
---
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] 17+ messages in thread