[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 03/11] xen/arm: introduce 1:1 direct-map for domUs
On 23/09/2021 08:11, Penny Zheng wrote:
From: Stefano Stabellini <stefano.stabellini@xxxxxxxxxx>
Cases where domU needs 1:1 direct-map memory map:
"1:1" and "direct-map" means pretty much the same. Given that the
property is name "direct-map", then I would drop "1:1".
* 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.
*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.
The two requirements are only necessary for device that are DMA-capable.
For device that can't do DMA, it will likely be fine to assign to
non-trusted guest.
Given that with direct-map, the IOMMU could be used but it is not required.
I can't parse it.
This commit avoids setting XEN_DOMCTL_CDF_iommu when the IOMMU is disabled and
direct_map is requested.
Since, for now, 1:1 direct-map is only supported when domain on Static
I think "Since" seems unnecessary.
Allocation, that is, "xen.static-mem" is defined in the domain configuration.
This commit also re-implements allocate_static_memory to allocate static memory
as guest RAM for 1:1 direct-map domain.
Signed-off-by: Stefano Stabellini <stefano.stabellini@xxxxxxxxxx>
Signed-off-by: Penny Zheng <penny.zheng@xxxxxxx>
---
docs/misc/arm/device-tree/booting.txt | 9 ++
xen/arch/arm/domain_build.c | 117 +++++++++++++++++---------
2 files changed, 85 insertions(+), 41 deletions(-)
diff --git a/docs/misc/arm/device-tree/booting.txt
b/docs/misc/arm/device-tree/booting.txt
index 44cd9e1a9a..3d637c747e 100644
--- a/docs/misc/arm/device-tree/booting.txt
+++ b/docs/misc/arm/device-tree/booting.txt
@@ -164,6 +164,15 @@ 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
+
+ Optional for Domain on Static Allocation.
+ An empty property to request the memory of the domain to be 1:1
+ direct-map(guest physical address == physical address).
+ WARNING: Users must be aware of this risk, that guests having access
+ to hardware with DMA capacity must be trusted, or it could use the
+ DMA engine to access any other memory area.
The WARNING is only applicable if the device is not protected by an
IOMMU. So this should be clarified because one may want still want to
use the direct-map (e.g. because the OS relies on the host layout) and
have IOMMU enabled.
+
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_build.c b/xen/arch/arm/domain_build.c
index 21d8a559af..213ad017dc 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -488,8 +488,14 @@ 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;
+
+ if ( !is_domain_direct_mapped(d) )
+ /* Infer next GFN when GFN != MFN. */
+ 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 )
@@ -537,14 +543,17 @@ static void __init allocate_static_memory(struct domain
*d,
}
This function was already pretty difficult to read. So I would rather
not add more complexity in it. Instead, I would look to split the common
code in a separate helper or possibly duplicate it.
reg_cells = addr_cells + size_cells;
- /*
- * The static memory will be mapped in the guest at the usual guest memory
- * addresses (GUEST_RAM0_BASE, GUEST_RAM1_BASE) defined by
- * xen/include/public/arch-arm.h.
- */
- gbank = 0;
- gsize = ramsize[gbank];
- kinfo->mem.bank[gbank].start = rambase[gbank];
+ if ( !is_domain_direct_mapped(d) )
+ {
+ /*
+ * The static memory will be mapped in the guest at the usual guest
+ * memory addresses (GUEST_RAM0_BASE, GUEST_RAM1_BASE) defined by
+ * xen/include/public/arch-arm.h.
+ */
+ 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));
@@ -572,42 +581,58 @@ static void __init allocate_static_memory(struct domain
*d,
printk(XENLOG_INFO "%pd: STATIC BANK[%u] %#"PRIpaddr"-%#"PRIpaddr"\n",
d, bank, pbase, pbase + psize);
- while ( 1 )
+ if ( !is_domain_direct_mapped(d) )
{
- /* Map as much as possible the static range to the guest bank */
- if ( !append_static_memory_to_bank(d, &kinfo->mem.bank[gbank],
smfn,
- min(psize, gsize)) )
- goto fail;
-
- /*
- * The current physical bank is fully mapped.
- * Handle the next physical bank.
- */
- if ( gsize >= psize )
+ while ( 1 )
{
- gsize = gsize - psize;
- break;
+ /* Map as much as possible the static range to the guest bank
*/
+ if ( !append_static_memory_to_bank(d, &kinfo->mem.bank[gbank],
+ smfn, min(psize, gsize)) )
+ goto fail;
+
+ /*
+ * The current physical bank is fully mapped.
+ * Handle the next physical bank.
+ */
+ if ( gsize >= psize )
+ {
+ gsize = gsize - psize;
+ break;
+ }
+ /*
+ * When current guest bank is not enough to map, exhaust
+ * the current one and seek to the next.
+ * Before seeking to the next, check if we still have available
+ * guest bank.
+ */
+ else if ( (gbank + 1) >= GUEST_RAM_BANKS )
+ {
+ printk(XENLOG_ERR "Exhausted all possible guest banks.\n");
+ goto fail;
+ }
+ else
+ {
+ psize = psize - gsize;
+ smfn = mfn_add(smfn, gsize >> PAGE_SHIFT);
+ /* Update to the next guest bank. */
+ gbank++;
+ gsize = ramsize[gbank];
+ kinfo->mem.bank[gbank].start = rambase[gbank];
+ }
}
+ }
+ else /* 1:1 direct-map. */
+ {
/*
- * When current guest bank is not enough to map, exhaust
- * the current one and seek to the next.
- * Before seeking to the next, check if we still have available
- * guest bank.
+ * One guest memory bank is matched with one physical
+ * memory bank.
*/
- else if ( (gbank + 1) >= GUEST_RAM_BANKS )
- {
- printk(XENLOG_ERR "Exhausted all possible guest banks.\n");
+ gbank = bank;
+ kinfo->mem.bank[gbank].start = pbase;
+
+ if ( !append_static_memory_to_bank(d, &kinfo->mem.bank[gbank],
+ smfn, psize) )
goto fail;
- }
- else
- {
- psize = psize - gsize;
- smfn = mfn_add(smfn, gsize >> PAGE_SHIFT);
- /* Update to the next guest bank. */
- gbank++;
- gsize = ramsize[gbank];
- kinfo->mem.bank[gbank].start = rambase[gbank];
- }
}
tot_size += psize;
@@ -2638,6 +2663,7 @@ void __init create_domUs(void)
{
struct dt_device_node *node;
const struct dt_device_node *chosen = dt_find_node_by_path("/chosen");
+ bool direct_map = false;
This is a bit redundant for just a couple of use. Instead, you could
directly use d_cfg.flags & XEN_DOMCTL_INTERNAL_directmap.
BUG_ON(chosen == NULL);
dt_for_each_child_node(chosen, node)
@@ -2658,8 +2684,17 @@ void __init create_domUs(void)
panic("Missing property 'cpus' for domain %s\n",
dt_node_name(node));
+ direct_map = dt_property_read_bool(node, "direct-map");
+ d_cfg.flags |= direct_map ? XEN_DOMCTL_INTERNAL_directmap : 0;
if ( dt_find_compatible_node(node, NULL, "multiboot,device-tree") )
- d_cfg.flags |= XEN_DOMCTL_CDF_iommu;
+ {
+ if ( iommu_enabled )
+ d_cfg.flags |= XEN_DOMCTL_CDF_iommu;
+ else if ( !direct_map )
+ panic("Assign a device but IOMMU and direct-map are all
disabled\n");
+ else
+ warning_add("Please be sure of having trusted guests, when doing
device assignment without IOMMU protection\n");
+ }
if ( !dt_property_read_u32(node, "nr_spis", &d_cfg.arch.nr_spis) )
{
Cheers,
--
Julien Grall
|
