Hi Stewart,
On 07/06/2023 04:02, Stewart Hildebrand wrote:
From: Oleksandr Tyshchenko <oleksandr_tyshche...@epam.com>
This flag will be re-used for PCI devices by the subsequent
patches.
I was expecting that we would only do PCI passthrough on boards where
all the PCI devices are behind an IOMMU. So it would be a all or nothing
and therefore is_protected would not be used.
Do you have an example where this wouldn't be the case?
Regardless that, it would be good to spell out that you also rename
helpers. But see below.
Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshche...@epam.com>
Signed-off-by: Stewart Hildebrand <stewart.hildebr...@amd.com>
---
v3->v4:
* move is_protected flag within struct device to reduce padding
* re-add device_is_protected checks in add_device hooks in
smmu-v3.c/ipmmu-vmsa.c
* split mmu-masters check into separate patch
v2->v3:
* no change
v1->v2:
* no change
downstream->v1:
* rebase
* s/dev_node->is_protected/dev_node->dev.is_protected/ in smmu.c
* s/dt_device_set_protected(dev_to_dt(dev))/device_set_protected(dev)/ in
smmu-v3.c
* remove redundant device_is_protected checks in smmu-v3.c/ipmmu-vmsa.c
(cherry picked from commit 59753aac77528a584d3950936b853ebf264b68e7 from
the downstream branch poc/pci-passthrough from
https://gitlab.com/xen-project/people/bmarquis/xen-arm-poc.git)
---
xen/arch/arm/domain_build.c | 4 ++--
xen/arch/arm/include/asm/device.h | 14 ++++++++++++++
xen/common/device_tree.c | 2 +-
xen/drivers/passthrough/arm/ipmmu-vmsa.c | 4 ++--
xen/drivers/passthrough/arm/smmu-v3.c | 5 +++--
xen/drivers/passthrough/arm/smmu.c | 2 +-
xen/drivers/passthrough/device_tree.c | 8 ++++----
xen/include/xen/device_tree.h | 13 -------------
8 files changed, 27 insertions(+), 25 deletions(-)
diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 3f4558ade67f..b229bfaae712 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -2524,7 +2524,7 @@ static int __init handle_device(struct domain *d, struct
dt_device_node *dev,
return res;
}
- if ( dt_device_is_protected(dev) )
+ if ( device_is_protected(dt_to_dev(dev)) )
I would actually prefer if we keep dt_device_is_protected() and call
device_is_protected(dt_to_dev(...)) within it.
{
dt_dprintk("%s setup iommu\n", dt_node_full_name(dev));
res = iommu_assign_dt_device(d, dev);
@@ -3024,7 +3024,7 @@ static int __init handle_passthrough_prop(struct
kernel_info *kinfo,
return res;
/* If xen_force, we allow assignment of devices without IOMMU protection. */
- if ( xen_force && !dt_device_is_protected(node) )
+ if ( xen_force && !device_is_protected(dt_to_dev(node)) )
return 0;
return iommu_assign_dt_device(kinfo->d, node);
diff --git a/xen/arch/arm/include/asm/device.h
b/xen/arch/arm/include/asm/device.h
index b5d451e08776..8ac807482737 100644
--- a/xen/arch/arm/include/asm/device.h
+++ b/xen/arch/arm/include/asm/device.h
@@ -1,6 +1,8 @@
#ifndef __ASM_ARM_DEVICE_H
#define __ASM_ARM_DEVICE_H
+#include <xen/types.h>
+
enum device_type
{
DEV_DT,
@@ -15,6 +17,8 @@ struct dev_archdata {
struct device
{
enum device_type type;
+ bool is_protected; /* Shows that device is protected by IOMMU */
+ uint8_t _pad[3];
AFAIK, a compiler would be allowed to use 8-byte for the enum. So the
padding would increase the size of the structure.
Therefore, I don't think you want to add an explicit padding here.
Cheers,
--
Julien Grall
