On 30/05/2023 15:10, Shameerali Kolothum Thodi wrote: >> -----Original Message----- >> From: Joao Martins [mailto:joao.m.martins@xxxxxxxxxx] >> Sent: 18 May 2023 21:47 >> To: iommu@xxxxxxxxxxxxxxx >> Cc: Jason Gunthorpe <jgg@xxxxxxxxxx>; Kevin Tian <kevin.tian@xxxxxxxxx>; >> Shameerali Kolothum Thodi <shameerali.kolothum.thodi@xxxxxxxxxx>; Lu >> Baolu <baolu.lu@xxxxxxxxxxxxxxx>; Yi Liu <yi.l.liu@xxxxxxxxx>; Yi Y Sun >> <yi.y.sun@xxxxxxxxx>; Eric Auger <eric.auger@xxxxxxxxxx>; Nicolin Chen >> <nicolinc@xxxxxxxxxx>; Joerg Roedel <joro@xxxxxxxxxx>; Jean-Philippe >> Brucker <jean-philippe@xxxxxxxxxx>; Suravee Suthikulpanit >> <suravee.suthikulpanit@xxxxxxx>; Will Deacon <will@xxxxxxxxxx>; Robin >> Murphy <robin.murphy@xxxxxxx>; Alex Williamson >> <alex.williamson@xxxxxxxxxx>; kvm@xxxxxxxxxxxxxxx; Joao Martins >> <joao.m.martins@xxxxxxxxxx> >> Subject: [PATCH RFCv2 24/24] iommu/arm-smmu-v3: Advertise >> IOMMU_DOMAIN_F_ENFORCE_DIRTY >> >> Now that we probe, and handle the DBM bit modifier, unblock >> the kAPI usage by exposing the IOMMU_DOMAIN_F_ENFORCE_DIRTY >> and implement it's requirement of revoking device attachment >> in the iommu_capable. Finally expose the IOMMU_CAP_DIRTY to >> users (IOMMUFD_DEVICE_GET_CAPS). >> >> Signed-off-by: Joao Martins <joao.m.martins@xxxxxxxxxx> >> --- >> drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 8 ++++++++ >> 1 file changed, 8 insertions(+) >> >> diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c >> b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c >> index bf0aac333725..71dd95a687fd 100644 >> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c >> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c >> @@ -2014,6 +2014,8 @@ static bool arm_smmu_capable(struct device *dev, >> enum iommu_cap cap) >> return master->smmu->features & >> ARM_SMMU_FEAT_COHERENCY; >> case IOMMU_CAP_NOEXEC: >> return true; >> + case IOMMU_CAP_DIRTY: >> + return arm_smmu_dbm_capable(master->smmu); >> default: >> return false; >> } >> @@ -2430,6 +2432,11 @@ static int arm_smmu_attach_dev(struct >> iommu_domain *domain, struct device *dev) >> master = dev_iommu_priv_get(dev); >> smmu = master->smmu; >> >> + if (domain->flags & IOMMU_DOMAIN_F_ENFORCE_DIRTY && >> + !arm_smmu_dbm_capable(smmu)) >> + return -EINVAL; >> + >> + > > Since we have the supported_flags always set to " IOMMU_DOMAIN_F_ENFORCE_DIRTY" > below, platforms that doesn't have DBM capability will fail here, right? > Or the idea is to set > domain flag only if the capability is reported true? But the iommu_domain_set_flags() doesn't > seems to check the capability though. > As posted the checking was only take place at device_attach (and you would set the enforcement flag if iommufd reports the capability for the device via IOMMU_DEVICE_GET_CAPS). But the workflow will change a bit: while the enforcement also takes place on device attach, but when we create a HWPT domain with flags (in domain_alloc_user[0]), the dirty tracking is also going to be checked there against the device passed in domain_alloc_user() in the driver implementation. And otherwise fail if doesn't support when dirty-tracking support enforcement as passed by flags. When we don't request dirty tracking the iommu ops that perform the dirty tracking will also be kept cleared. [0] https://lore.kernel.org/linux-iommu/20230511143844.22693-2-yi.l.liu@xxxxxxxxx/ > (This seems to be causing problem with a rebased Qemu branch for ARM I have while sanity > testing on a platform that doesn't have DBM. I need to double check though). > Perhaps due to the broken check I had that I need validate the two bits together, when it didn't had DBM set? Or I suspect because the qemu last patch I was always end up setting IOMMU_DOMAIN_F_ENFORCE_DIRTY [*], and because the checking is always enabled you can never attach devices. [*] That last patch isn't quite there yet as it is meant to be using device-get-caps prior to setting the enforcement, like the selftests
