> -----Original Message----- > From: Joao Martins [mailto:joao.m.martins@xxxxxxxxxx] > Sent: 30 May 2023 20:20 > To: Shameerali Kolothum Thodi <shameerali.kolothum.thodi@xxxxxxxxxx> > Cc: Jason Gunthorpe <jgg@xxxxxxxxxx>; Kevin Tian <kevin.tian@xxxxxxxxx>; > 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; > iommu@xxxxxxxxxxxxxxx > Subject: Re: [PATCH RFCv2 24/24] iommu/arm-smmu-v3: Advertise > IOMMU_DOMAIN_F_ENFORCE_DIRTY > > 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). Ok. So CAPS is retrieved before we set the enforcement flag. > > 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. Ok. > > [0] > https://lore.kernel.org/linux-iommu/20230511143844.22693-2-yi.l.liu@inte > l.com/ > > > (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? I have that fixed in my branch now. 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. Ah.. this is it. > [*] 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 Got it. Thanks, Shameer
