| Message ID | 20240222094923.33104-2-shameerali.kolothum.thodi@huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | iommu/smmuv3: Add IOMMUFD dirty tracking support for SMMUv3 | expand |
Hi, I'm aiming to (slowly) get more involved with SMMU activities, although I'm sure it will take a while to get up to speed and provide useful input. It was suggested that this series would be a useful starting point to dip my toe in. Please bear with me while I ask stupid questions... On 22/02/2024 09:49, Shameer Kolothum wrote: > From: Jean-Philippe Brucker <jean-philippe@linaro.org> > > If the SMMU supports it and the kernel was built with HTTU support, > Probe support for Hardware Translation Table Update (HTTU) which is > essentially to enable hardware update of access and dirty flags. > > Probe and set the smmu::features for Hardware Dirty and Hardware Access > bits. This is in preparation, to enable it on the context descriptors of > stage 1 format. > > Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org> > Signed-off-by: Joao Martins <joao.m.martins@oracle.com> > Reviewed-by: Jason Gunthorpe <jgg@nvidia.com> > Signed-off-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com> Except for the nit (feel free to ignore it), LGTM! Reviewed-by: Ryan Roberts <ryan.roberts@arm.com> > --- > drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 32 +++++++++++++++++++++ > drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h | 5 ++++ > 2 files changed, 37 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 0606166a8781..bd30739e3588 100644 > --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c > +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c > @@ -4196,6 +4196,28 @@ static void arm_smmu_device_iidr_probe(struct arm_smmu_device *smmu) > } > } > > +static void arm_smmu_get_httu(struct arm_smmu_device *smmu, u32 reg) > +{ > + u32 fw_features = smmu->features & (ARM_SMMU_FEAT_HA | ARM_SMMU_FEAT_HD); > + u32 features = 0; > + > + switch (FIELD_GET(IDR0_HTTU, reg)) { > + case IDR0_HTTU_ACCESS_DIRTY: > + features |= ARM_SMMU_FEAT_HD; > + fallthrough; > + case IDR0_HTTU_ACCESS: > + features |= ARM_SMMU_FEAT_HA; > + } > + > + if (smmu->dev->of_node) > + smmu->features |= features; > + else if (features != fw_features) > + /* ACPI IORT sets the HTTU bits */ > + dev_warn(smmu->dev, > + "IDR0.HTTU overridden by FW configuration (0x%x)\n", > + fw_features); > +} > + > static int arm_smmu_device_hw_probe(struct arm_smmu_device *smmu) > { > u32 reg; > @@ -4256,6 +4278,8 @@ static int arm_smmu_device_hw_probe(struct arm_smmu_device *smmu) > smmu->features |= ARM_SMMU_FEAT_E2H; > } > > + arm_smmu_get_httu(smmu, reg); > + > /* > * The coherency feature as set by FW is used in preference to the ID > * register, but warn on mismatch. > @@ -4448,6 +4472,14 @@ static int arm_smmu_device_acpi_probe(struct platform_device *pdev, > if (iort_smmu->flags & ACPI_IORT_SMMU_V3_COHACC_OVERRIDE) > smmu->features |= ARM_SMMU_FEAT_COHERENCY; > > + switch (FIELD_GET(ACPI_IORT_SMMU_V3_HTTU_OVERRIDE, iort_smmu->flags)) { > + case IDR0_HTTU_ACCESS_DIRTY: > + smmu->features |= ARM_SMMU_FEAT_HD; > + fallthrough; > + case IDR0_HTTU_ACCESS: > + smmu->features |= ARM_SMMU_FEAT_HA; > + } > + > return 0; > } > #else > diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h > index 45bcd72fcda4..5e51a6c1d55f 100644 > --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h > +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h > @@ -33,6 +33,9 @@ > #define IDR0_ASID16 (1 << 12) > #define IDR0_ATS (1 << 10) > #define IDR0_HYP (1 << 9) > +#define IDR0_HTTU GENMASK(7, 6) > +#define IDR0_HTTU_ACCESS 1 > +#define IDR0_HTTU_ACCESS_DIRTY 2 > #define IDR0_COHACC (1 << 4) > #define IDR0_TTF GENMASK(3, 2) > #define IDR0_TTF_AARCH64 2 > @@ -668,6 +671,8 @@ struct arm_smmu_device { > #define ARM_SMMU_FEAT_SVA (1 << 17) > #define ARM_SMMU_FEAT_E2H (1 << 18) > #define ARM_SMMU_FEAT_NESTING (1 << 19) > +#define ARM_SMMU_FEAT_HA (1 << 20) > +#define ARM_SMMU_FEAT_HD (1 << 21) nit: HA and HD are a bit opaque, at least to me. I guess they are HW Access and HW Dirty? Perhaps ARM_SMMU_FEAT_HW_ACCESS and ARM_SMMU_FEAT_HW_DIRTY are more expressive? > u32 features; > > #define ARM_SMMU_OPT_SKIP_PREFETCH (1 << 0)
On Tue, Apr 23, 2024 at 03:41:30PM +0100, Ryan Roberts wrote: > Hi, > > I'm aiming to (slowly) get more involved with SMMU activities, although I'm sure > it will take a while to get up to speed and provide useful input. It was > suggested that this series would be a useful starting point to dip my toe in. > Please bear with me while I ask stupid questions... Nice! I would like to see this merged as it was part of the original three implementations for iommufd dirty tracking. The other two have been merged for a long time now.. Jason
Hi, > -----Original Message----- > From: Ryan Roberts <ryan.roberts@arm.com> > Sent: Tuesday, April 23, 2024 3:42 PM > To: Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com>; > iommu@lists.linux.dev; linux-arm-kernel@lists.infradead.org > Cc: joro@8bytes.org; jgg@nvidia.com; kevin.tian@intel.com; > nicolinc@nvidia.com; mshavit@google.com; robin.murphy@arm.com; > will@kernel.org; joao.m.martins@oracle.com; jiangkunkun > <jiangkunkun@huawei.com>; zhukeqian <zhukeqian1@huawei.com>; > Linuxarm <linuxarm@huawei.com> > Subject: Re: [PATCH v2 1/4] iommu/arm-smmu-v3: Add feature detection for > HTTU > > Hi, > > I'm aiming to (slowly) get more involved with SMMU activities, although I'm > sure > it will take a while to get up to speed and provide useful input. It was > suggested that this series would be a useful starting point to dip my toe in. > Please bear with me while I ask stupid questions... Thanks for going through the series. I am planning to respin this one soon, once Jason's SMMUv3 refactor series in some good form. > > > On 22/02/2024 09:49, Shameer Kolothum wrote: > > From: Jean-Philippe Brucker <jean-philippe@linaro.org> > > > > If the SMMU supports it and the kernel was built with HTTU support, > > Probe support for Hardware Translation Table Update (HTTU) which is > > essentially to enable hardware update of access and dirty flags. > > > > Probe and set the smmu::features for Hardware Dirty and Hardware Access > > bits. This is in preparation, to enable it on the context descriptors of > > stage 1 format. > > > > Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org> > > Signed-off-by: Joao Martins <joao.m.martins@oracle.com> > > Reviewed-by: Jason Gunthorpe <jgg@nvidia.com> > > Signed-off-by: Shameer Kolothum > <shameerali.kolothum.thodi@huawei.com> > > Except for the nit (feel free to ignore it), LGTM! > > Reviewed-by: Ryan Roberts <ryan.roberts@arm.com> > > > > --- > > drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 32 > +++++++++++++++++++++ > > drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h | 5 ++++ > > 2 files changed, 37 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 0606166a8781..bd30739e3588 100644 > > --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c > > +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c > > @@ -4196,6 +4196,28 @@ static void arm_smmu_device_iidr_probe(struct > arm_smmu_device *smmu) > > } > > } > > > > +static void arm_smmu_get_httu(struct arm_smmu_device *smmu, u32 > reg) > > +{ > > + u32 fw_features = smmu->features & (ARM_SMMU_FEAT_HA | > ARM_SMMU_FEAT_HD); > > + u32 features = 0; > > + > > + switch (FIELD_GET(IDR0_HTTU, reg)) { > > + case IDR0_HTTU_ACCESS_DIRTY: > > + features |= ARM_SMMU_FEAT_HD; > > + fallthrough; > > + case IDR0_HTTU_ACCESS: > > + features |= ARM_SMMU_FEAT_HA; > > + } > > + > > + if (smmu->dev->of_node) > > + smmu->features |= features; > > + else if (features != fw_features) > > + /* ACPI IORT sets the HTTU bits */ > > + dev_warn(smmu->dev, > > + "IDR0.HTTU overridden by FW configuration > (0x%x)\n", > > + fw_features); > > +} > > + > > static int arm_smmu_device_hw_probe(struct arm_smmu_device *smmu) > > { > > u32 reg; > > @@ -4256,6 +4278,8 @@ static int arm_smmu_device_hw_probe(struct > arm_smmu_device *smmu) > > smmu->features |= ARM_SMMU_FEAT_E2H; > > } > > > > + arm_smmu_get_httu(smmu, reg); > > + > > /* > > * The coherency feature as set by FW is used in preference to the ID > > * register, but warn on mismatch. > > @@ -4448,6 +4472,14 @@ static int arm_smmu_device_acpi_probe(struct > platform_device *pdev, > > if (iort_smmu->flags & ACPI_IORT_SMMU_V3_COHACC_OVERRIDE) > > smmu->features |= ARM_SMMU_FEAT_COHERENCY; > > > > + switch (FIELD_GET(ACPI_IORT_SMMU_V3_HTTU_OVERRIDE, > iort_smmu->flags)) { > > + case IDR0_HTTU_ACCESS_DIRTY: > > + smmu->features |= ARM_SMMU_FEAT_HD; > > + fallthrough; > > + case IDR0_HTTU_ACCESS: > > + smmu->features |= ARM_SMMU_FEAT_HA; > > + } > > + > > return 0; > > } > > #else > > diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h > b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h > > index 45bcd72fcda4..5e51a6c1d55f 100644 > > --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h > > +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h > > @@ -33,6 +33,9 @@ > > #define IDR0_ASID16 (1 << 12) > > #define IDR0_ATS (1 << 10) > > #define IDR0_HYP (1 << 9) > > +#define IDR0_HTTU GENMASK(7, 6) > > +#define IDR0_HTTU_ACCESS 1 > > +#define IDR0_HTTU_ACCESS_DIRTY 2 > > #define IDR0_COHACC (1 << 4) > > #define IDR0_TTF GENMASK(3, 2) > > #define IDR0_TTF_AARCH64 2 > > @@ -668,6 +671,8 @@ struct arm_smmu_device { > > #define ARM_SMMU_FEAT_SVA (1 << 17) > > #define ARM_SMMU_FEAT_E2H (1 << 18) > > #define ARM_SMMU_FEAT_NESTING (1 << 19) > > +#define ARM_SMMU_FEAT_HA (1 << 20) > > +#define ARM_SMMU_FEAT_HD (1 << 21) > > nit: HA and HD are a bit opaque, at least to me. I guess they are HW Access > and > HW Dirty? Perhaps ARM_SMMU_FEAT_HW_ACCESS and > ARM_SMMU_FEAT_HW_DIRTY are more > expressive? Nothing against it. Just that _HW_ is not used in driver anywhere(AFAICS) and HA/HD are used in the specification. How about we rename to ARM_SMMU_FEAT_HTTU_HA and ARM_SMMU_FEAT_HTTU_HA? Thanks, Shameer > > > u32 features; > > > > #define ARM_SMMU_OPT_SKIP_PREFETCH (1 << 0) >
On 24/04/2024 09:01, Shameerali Kolothum Thodi wrote: > Hi, > >> -----Original Message----- >> From: Ryan Roberts <ryan.roberts@arm.com> >> Sent: Tuesday, April 23, 2024 3:42 PM >> To: Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com>; >> iommu@lists.linux.dev; linux-arm-kernel@lists.infradead.org >> Cc: joro@8bytes.org; jgg@nvidia.com; kevin.tian@intel.com; >> nicolinc@nvidia.com; mshavit@google.com; robin.murphy@arm.com; >> will@kernel.org; joao.m.martins@oracle.com; jiangkunkun >> <jiangkunkun@huawei.com>; zhukeqian <zhukeqian1@huawei.com>; >> Linuxarm <linuxarm@huawei.com> >> Subject: Re: [PATCH v2 1/4] iommu/arm-smmu-v3: Add feature detection for >> HTTU >> >> Hi, >> >> I'm aiming to (slowly) get more involved with SMMU activities, although I'm >> sure >> it will take a while to get up to speed and provide useful input. It was >> suggested that this series would be a useful starting point to dip my toe in. >> Please bear with me while I ask stupid questions... > > Thanks for going through the series. I am planning to respin this one soon, once > Jason's SMMUv3 refactor series in some good form. > >> >> >> On 22/02/2024 09:49, Shameer Kolothum wrote: >>> From: Jean-Philippe Brucker <jean-philippe@linaro.org> >>> >>> If the SMMU supports it and the kernel was built with HTTU support, >>> Probe support for Hardware Translation Table Update (HTTU) which is >>> essentially to enable hardware update of access and dirty flags. >>> >>> Probe and set the smmu::features for Hardware Dirty and Hardware Access >>> bits. This is in preparation, to enable it on the context descriptors of >>> stage 1 format. >>> >>> Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org> >>> Signed-off-by: Joao Martins <joao.m.martins@oracle.com> >>> Reviewed-by: Jason Gunthorpe <jgg@nvidia.com> >>> Signed-off-by: Shameer Kolothum >> <shameerali.kolothum.thodi@huawei.com> >> >> Except for the nit (feel free to ignore it), LGTM! >> >> Reviewed-by: Ryan Roberts <ryan.roberts@arm.com> >> >> >>> --- >>> drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 32 >> +++++++++++++++++++++ >>> drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h | 5 ++++ >>> 2 files changed, 37 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 0606166a8781..bd30739e3588 100644 >>> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c >>> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c >>> @@ -4196,6 +4196,28 @@ static void arm_smmu_device_iidr_probe(struct >> arm_smmu_device *smmu) >>> } >>> } >>> >>> +static void arm_smmu_get_httu(struct arm_smmu_device *smmu, u32 >> reg) >>> +{ >>> + u32 fw_features = smmu->features & (ARM_SMMU_FEAT_HA | >> ARM_SMMU_FEAT_HD); >>> + u32 features = 0; >>> + >>> + switch (FIELD_GET(IDR0_HTTU, reg)) { >>> + case IDR0_HTTU_ACCESS_DIRTY: >>> + features |= ARM_SMMU_FEAT_HD; >>> + fallthrough; >>> + case IDR0_HTTU_ACCESS: >>> + features |= ARM_SMMU_FEAT_HA; >>> + } >>> + >>> + if (smmu->dev->of_node) >>> + smmu->features |= features; >>> + else if (features != fw_features) >>> + /* ACPI IORT sets the HTTU bits */ >>> + dev_warn(smmu->dev, >>> + "IDR0.HTTU overridden by FW configuration >> (0x%x)\n", >>> + fw_features); >>> +} >>> + >>> static int arm_smmu_device_hw_probe(struct arm_smmu_device *smmu) >>> { >>> u32 reg; >>> @@ -4256,6 +4278,8 @@ static int arm_smmu_device_hw_probe(struct >> arm_smmu_device *smmu) >>> smmu->features |= ARM_SMMU_FEAT_E2H; >>> } >>> >>> + arm_smmu_get_httu(smmu, reg); >>> + >>> /* >>> * The coherency feature as set by FW is used in preference to the ID >>> * register, but warn on mismatch. >>> @@ -4448,6 +4472,14 @@ static int arm_smmu_device_acpi_probe(struct >> platform_device *pdev, >>> if (iort_smmu->flags & ACPI_IORT_SMMU_V3_COHACC_OVERRIDE) >>> smmu->features |= ARM_SMMU_FEAT_COHERENCY; >>> >>> + switch (FIELD_GET(ACPI_IORT_SMMU_V3_HTTU_OVERRIDE, >> iort_smmu->flags)) { >>> + case IDR0_HTTU_ACCESS_DIRTY: >>> + smmu->features |= ARM_SMMU_FEAT_HD; >>> + fallthrough; >>> + case IDR0_HTTU_ACCESS: >>> + smmu->features |= ARM_SMMU_FEAT_HA; >>> + } >>> + >>> return 0; >>> } >>> #else >>> diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h >> b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h >>> index 45bcd72fcda4..5e51a6c1d55f 100644 >>> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h >>> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h >>> @@ -33,6 +33,9 @@ >>> #define IDR0_ASID16 (1 << 12) >>> #define IDR0_ATS (1 << 10) >>> #define IDR0_HYP (1 << 9) >>> +#define IDR0_HTTU GENMASK(7, 6) >>> +#define IDR0_HTTU_ACCESS 1 >>> +#define IDR0_HTTU_ACCESS_DIRTY 2 >>> #define IDR0_COHACC (1 << 4) >>> #define IDR0_TTF GENMASK(3, 2) >>> #define IDR0_TTF_AARCH64 2 >>> @@ -668,6 +671,8 @@ struct arm_smmu_device { >>> #define ARM_SMMU_FEAT_SVA (1 << 17) >>> #define ARM_SMMU_FEAT_E2H (1 << 18) >>> #define ARM_SMMU_FEAT_NESTING (1 << 19) >>> +#define ARM_SMMU_FEAT_HA (1 << 20) >>> +#define ARM_SMMU_FEAT_HD (1 << 21) >> >> nit: HA and HD are a bit opaque, at least to me. I guess they are HW Access >> and >> HW Dirty? Perhaps ARM_SMMU_FEAT_HW_ACCESS and >> ARM_SMMU_FEAT_HW_DIRTY are more >> expressive? > > Nothing against it. Just that _HW_ is not used in driver anywhere(AFAICS) > and HA/HD are used in the specification. Ahh yes, I see that now. > How about we rename to > ARM_SMMU_FEAT_HTTU_HA and ARM_SMMU_FEAT_HTTU_HA? Yes, that's much more obvious from where I'm standing. But given HA and HD are fields in the spec, I'm also fine with leaving as is. Up to you. > > Thanks, > Shameer > >> >>> u32 features; >>> >>> #define ARM_SMMU_OPT_SKIP_PREFETCH (1 << 0) >> >
On 23/04/2024 15:52, Jason Gunthorpe wrote: > On Tue, Apr 23, 2024 at 03:41:30PM +0100, Ryan Roberts wrote: >> Hi, >> >> I'm aiming to (slowly) get more involved with SMMU activities, although I'm sure >> it will take a while to get up to speed and provide useful input. It was >> suggested that this series would be a useful starting point to dip my toe in. >> Please bear with me while I ask stupid questions... > > Nice! > > I would like to see this merged as it was part of the original three > implementations for iommufd dirty tracking. The other two have been > merged for a long time now.. My understanding is that this series should pretty much apply on top of mainline, is that correct? (in practice there are some conflicts, but I think they are trivial). I don't think it depends on anything new in your smmuv3_newapi branch? Could you point me to any series you have on list that I could help with review on? > > Jason
On Wed, Apr 24, 2024 at 11:04:43AM +0100, Ryan Roberts wrote: > On 23/04/2024 15:52, Jason Gunthorpe wrote: > > On Tue, Apr 23, 2024 at 03:41:30PM +0100, Ryan Roberts wrote: > >> Hi, > >> > >> I'm aiming to (slowly) get more involved with SMMU activities, although I'm sure > >> it will take a while to get up to speed and provide useful input. It was > >> suggested that this series would be a useful starting point to dip my toe in. > >> Please bear with me while I ask stupid questions... > > > > Nice! > > > > I would like to see this merged as it was part of the original three > > implementations for iommufd dirty tracking. The other two have been > > merged for a long time now.. > > My understanding is that this series should pretty much apply on top of > mainline, is that correct? (in practice there are some conflicts, but I think > they are trivial). It was originally based on my part 2, but I suspect it could apply easially on top of part 2a. I don't think it is worth rebasing to v6.9-rc since I expect Will to take 2a. > I don't think it depends on anything new in your smmuv3_newapi branch? I feel part 2a is done, hopefully Will will take it soon: https://lore.kernel.org/linux-iommu/0-v8-4c4298c63951+13484-smmuv3_newapi_p2_jgg@nvidia.com/ There are quite a few distros waiting on this: https://lore.kernel.org/linux-iommu/cover.1712977210.git.nicolinc@nvidia.com/ The cmdq area is not something I've studied deeply I will be reposting the latter half of this as a part 2b: https://lore.kernel.org/linux-iommu/0-v6-228e7adf25eb+4155-smmuv3_newapi_p2_jgg@nvidia.com/ Which is the prerequisite for the part 3 series to enable nested translation an vSMMUv3 for iommufd. Nested translation and a few more bits is prerequisite to finally enable BTM: https://lore.kernel.org/linux-iommu/20240208151837.35068-1-shameerali.kolothum.thodi@huawei.com/ Thanks, Jason
On 24/04/2024 13:23, Jason Gunthorpe wrote: > On Wed, Apr 24, 2024 at 11:04:43AM +0100, Ryan Roberts wrote: >> On 23/04/2024 15:52, Jason Gunthorpe wrote: >>> On Tue, Apr 23, 2024 at 03:41:30PM +0100, Ryan Roberts wrote: >>>> Hi, >>>> >>>> I'm aiming to (slowly) get more involved with SMMU activities, although I'm sure >>>> it will take a while to get up to speed and provide useful input. It was >>>> suggested that this series would be a useful starting point to dip my toe in. >>>> Please bear with me while I ask stupid questions... >>> >>> Nice! >>> >>> I would like to see this merged as it was part of the original three >>> implementations for iommufd dirty tracking. The other two have been >>> merged for a long time now.. >> >> My understanding is that this series should pretty much apply on top of >> mainline, is that correct? (in practice there are some conflicts, but I think >> they are trivial). > > It was originally based on my part 2, but I suspect it could apply > easially on top of part 2a. I don't think it is worth rebasing to > v6.9-rc since I expect Will to take 2a. > >> I don't think it depends on anything new in your smmuv3_newapi branch? > > I feel part 2a is done, hopefully Will will take it soon: > > https://lore.kernel.org/linux-iommu/0-v8-4c4298c63951+13484-smmuv3_newapi_p2_jgg@nvidia.com/ > > There are quite a few distros waiting on this: > > https://lore.kernel.org/linux-iommu/cover.1712977210.git.nicolinc@nvidia.com/ Great thanks for the links. I'll aim to start working through from this one onwards as an when I can. > > The cmdq area is not something I've studied deeply > > I will be reposting the latter half of this as a part 2b: > > https://lore.kernel.org/linux-iommu/0-v6-228e7adf25eb+4155-smmuv3_newapi_p2_jgg@nvidia.com/ > > Which is the prerequisite for the part 3 series to enable nested > translation an vSMMUv3 for iommufd. > > Nested translation and a few more bits is prerequisite to finally enable BTM: > > https://lore.kernel.org/linux-iommu/20240208151837.35068-1-shameerali.kolothum.thodi@huawei.com/ > > Thanks, > Jason
> -----Original Message----- > From: Jason Gunthorpe <jgg@nvidia.com> > Sent: Wednesday, April 24, 2024 1:24 PM > To: Ryan Roberts <ryan.roberts@arm.com> > Cc: Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com>; > iommu@lists.linux.dev; linux-arm-kernel@lists.infradead.org; joro@8bytes.org; > kevin.tian@intel.com; nicolinc@nvidia.com; mshavit@google.com; > robin.murphy@arm.com; will@kernel.org; joao.m.martins@oracle.com; > jiangkunkun <jiangkunkun@huawei.com>; zhukeqian > <zhukeqian1@huawei.com>; Linuxarm <linuxarm@huawei.com> > Subject: Re: [PATCH v2 1/4] iommu/arm-smmu-v3: Add feature detection for > HTTU > > On Wed, Apr 24, 2024 at 11:04:43AM +0100, Ryan Roberts wrote: > > On 23/04/2024 15:52, Jason Gunthorpe wrote: > > > On Tue, Apr 23, 2024 at 03:41:30PM +0100, Ryan Roberts wrote: > > >> Hi, > > >> > > >> I'm aiming to (slowly) get more involved with SMMU activities, although I'm > sure > > >> it will take a while to get up to speed and provide useful input. It was > > >> suggested that this series would be a useful starting point to dip my toe in. > > >> Please bear with me while I ask stupid questions... > > > > > > Nice! > > > > > > I would like to see this merged as it was part of the original three > > > implementations for iommufd dirty tracking. The other two have been > > > merged for a long time now.. > > > > My understanding is that this series should pretty much apply on top of > > mainline, is that correct? (in practice there are some conflicts, but I think > > they are trivial). > > It was originally based on my part 2, but I suspect it could apply > easially on top of part 2a. I don't think it is worth rebasing to > v6.9-rc since I expect Will to take 2a. But isn't arm_smmu_domain_alloc_user() gets introduced only in part 3 of the series? Do we plan to support DIRTY_TRACKING with domain_alloc( ) itself by default? Thanks, Shameer
On Wed, Apr 24, 2024 at 01:20:53PM +0000, Shameerali Kolothum Thodi wrote: > > On Wed, Apr 24, 2024 at 11:04:43AM +0100, Ryan Roberts wrote: > > > On 23/04/2024 15:52, Jason Gunthorpe wrote: > > > > On Tue, Apr 23, 2024 at 03:41:30PM +0100, Ryan Roberts wrote: > > > >> Hi, > > > >> > > > >> I'm aiming to (slowly) get more involved with SMMU activities, although I'm > > sure > > > >> it will take a while to get up to speed and provide useful input. It was > > > >> suggested that this series would be a useful starting point to dip my toe in. > > > >> Please bear with me while I ask stupid questions... > > > > > > > > Nice! > > > > > > > > I would like to see this merged as it was part of the original three > > > > implementations for iommufd dirty tracking. The other two have been > > > > merged for a long time now.. > > > > > > My understanding is that this series should pretty much apply on top of > > > mainline, is that correct? (in practice there are some conflicts, but I think > > > they are trivial). > > > > It was originally based on my part 2, but I suspect it could apply > > easially on top of part 2a. I don't think it is worth rebasing to > > v6.9-rc since I expect Will to take 2a. > > But isn't arm_smmu_domain_alloc_user() gets introduced only in part 3 > of the series? Right. "easially" was perhaps a bit too strong. It would have to be rebased and pull back a little bit of the domain_alloc_user() implementation. If it is good otherwise lets consider doing this. Maybe HTTU and part 2b can go together during the v6.11 cycle? > Do we plan to support DIRTY_TRACKING with domain_alloc( ) > itself by default? No Jason
> -----Original Message----- > From: Jason Gunthorpe <jgg@nvidia.com> > Sent: Wednesday, April 24, 2024 2:32 PM > To: Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com> > Cc: Ryan Roberts <ryan.roberts@arm.com>; iommu@lists.linux.dev; linux-arm- > kernel@lists.infradead.org; joro@8bytes.org; kevin.tian@intel.com; > nicolinc@nvidia.com; mshavit@google.com; robin.murphy@arm.com; > will@kernel.org; joao.m.martins@oracle.com; jiangkunkun > <jiangkunkun@huawei.com>; zhukeqian <zhukeqian1@huawei.com>; Linuxarm > <linuxarm@huawei.com> > Subject: Re: [PATCH v2 1/4] iommu/arm-smmu-v3: Add feature detection for > HTTU > > On Wed, Apr 24, 2024 at 01:20:53PM +0000, Shameerali Kolothum Thodi wrote: > > > > On Wed, Apr 24, 2024 at 11:04:43AM +0100, Ryan Roberts wrote: > > > > On 23/04/2024 15:52, Jason Gunthorpe wrote: > > > > > On Tue, Apr 23, 2024 at 03:41:30PM +0100, Ryan Roberts wrote: > > > > >> Hi, > > > > >> > > > > >> I'm aiming to (slowly) get more involved with SMMU activities, although > I'm > > > sure > > > > >> it will take a while to get up to speed and provide useful input. It was > > > > >> suggested that this series would be a useful starting point to dip my toe > in. > > > > >> Please bear with me while I ask stupid questions... > > > > > > > > > > Nice! > > > > > > > > > > I would like to see this merged as it was part of the original three > > > > > implementations for iommufd dirty tracking. The other two have been > > > > > merged for a long time now.. > > > > > > > > My understanding is that this series should pretty much apply on top of > > > > mainline, is that correct? (in practice there are some conflicts, but I think > > > > they are trivial). > > > > > > It was originally based on my part 2, but I suspect it could apply > > > easially on top of part 2a. I don't think it is worth rebasing to > > > v6.9-rc since I expect Will to take 2a. > > > > But isn't arm_smmu_domain_alloc_user() gets introduced only in part 3 > > of the series? > > Right. "easially" was perhaps a bit too strong. It would have to be > rebased and pull back a little bit of the domain_alloc_user() > implementation. > > If it is good otherwise lets consider doing this. Maybe HTTU and part > 2b can go together during the v6.11 cycle? Sure. Let us target that. Thanks, Shameer
On Wed, Apr 24, 2024 at 01:43:33PM +0000, Shameerali Kolothum Thodi wrote: > > Right. "easially" was perhaps a bit too strong. It would have to be > > rebased and pull back a little bit of the domain_alloc_user() > > implementation. > > > > If it is good otherwise lets consider doing this. Maybe HTTU and part > > 2b can go together during the v6.11 cycle? > > Sure. Let us target that. Okay, soonly post a v3 on top of the latest 2b patch with the comments addressed and lets see. Jason
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 0606166a8781..bd30739e3588 100644 --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c @@ -4196,6 +4196,28 @@ static void arm_smmu_device_iidr_probe(struct arm_smmu_device *smmu) } } +static void arm_smmu_get_httu(struct arm_smmu_device *smmu, u32 reg) +{ + u32 fw_features = smmu->features & (ARM_SMMU_FEAT_HA | ARM_SMMU_FEAT_HD); + u32 features = 0; + + switch (FIELD_GET(IDR0_HTTU, reg)) { + case IDR0_HTTU_ACCESS_DIRTY: + features |= ARM_SMMU_FEAT_HD; + fallthrough; + case IDR0_HTTU_ACCESS: + features |= ARM_SMMU_FEAT_HA; + } + + if (smmu->dev->of_node) + smmu->features |= features; + else if (features != fw_features) + /* ACPI IORT sets the HTTU bits */ + dev_warn(smmu->dev, + "IDR0.HTTU overridden by FW configuration (0x%x)\n", + fw_features); +} + static int arm_smmu_device_hw_probe(struct arm_smmu_device *smmu) { u32 reg; @@ -4256,6 +4278,8 @@ static int arm_smmu_device_hw_probe(struct arm_smmu_device *smmu) smmu->features |= ARM_SMMU_FEAT_E2H; } + arm_smmu_get_httu(smmu, reg); + /* * The coherency feature as set by FW is used in preference to the ID * register, but warn on mismatch. @@ -4448,6 +4472,14 @@ static int arm_smmu_device_acpi_probe(struct platform_device *pdev, if (iort_smmu->flags & ACPI_IORT_SMMU_V3_COHACC_OVERRIDE) smmu->features |= ARM_SMMU_FEAT_COHERENCY; + switch (FIELD_GET(ACPI_IORT_SMMU_V3_HTTU_OVERRIDE, iort_smmu->flags)) { + case IDR0_HTTU_ACCESS_DIRTY: + smmu->features |= ARM_SMMU_FEAT_HD; + fallthrough; + case IDR0_HTTU_ACCESS: + smmu->features |= ARM_SMMU_FEAT_HA; + } + return 0; } #else diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h index 45bcd72fcda4..5e51a6c1d55f 100644 --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h @@ -33,6 +33,9 @@ #define IDR0_ASID16 (1 << 12) #define IDR0_ATS (1 << 10) #define IDR0_HYP (1 << 9) +#define IDR0_HTTU GENMASK(7, 6) +#define IDR0_HTTU_ACCESS 1 +#define IDR0_HTTU_ACCESS_DIRTY 2 #define IDR0_COHACC (1 << 4) #define IDR0_TTF GENMASK(3, 2) #define IDR0_TTF_AARCH64 2 @@ -668,6 +671,8 @@ struct arm_smmu_device { #define ARM_SMMU_FEAT_SVA (1 << 17) #define ARM_SMMU_FEAT_E2H (1 << 18) #define ARM_SMMU_FEAT_NESTING (1 << 19) +#define ARM_SMMU_FEAT_HA (1 << 20) +#define ARM_SMMU_FEAT_HD (1 << 21) u32 features; #define ARM_SMMU_OPT_SKIP_PREFETCH (1 << 0)