* Re: [PATCH v3 00/13] Add support for Hisilicon SMMU architecture [not found] ` <53CC7234.4080703-hv44wF8Li93QT0dZR+AlfA@public.gmane.org> @ 2014-07-21 9:39 ` Will Deacon 2014-07-22 3:54 ` leizhen 0 siblings, 1 reply; 2+ messages in thread From: Will Deacon @ 2014-07-21 9:39 UTC (permalink / raw) To: leizhen Cc: huxinwei-hv44wF8Li93QT0dZR+AlfA@public.gmane.org, iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA, Lizefan, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r [re-adding the lists] On Mon, Jul 21, 2014 at 02:51:48AM +0100, leizhen wrote: > Hi, Will Hello, > I have sent this patch a week ago. I saw that you and Mitchel Humpherys had > sent some patches which will impact my code. I list below: > iommu/arm-smmu: remove support for chained SMMUs > iommu/arm-smmu: fix some checkpatch issues There's also all of the development work I'm doing for VFIO and the page-table rework that Varun's working on, so these patches are going to be difficult to merge if we ever get to that point. I think Mitchel and Olav are also busy trying to get their SMMU supported by the driver. Given that they've actually bothered to follow the architecture, I don't see why they should be messed about by your patches causing conflicts all over the place. > I can ajust my patch because of above. I known this patch is not follow what > you suggested before(fit into arm-smmu.c). But I found when we need to support > ARM SMMUv3, maybe we should do like this. I really want to know whether or not > you agree the software framework in this patch? Or what do you think about > SMMUv3? The only code I foresee sharing between SMMUv2 and SMMUv3 is the page-table creation code. The two architectures are significantly different, so I don't think your split really helps us there. For example, you've left the probing code in smmu-base.c. Of course, if HiSilicon follow the SMMUv3 spec as well they did with SMMUv2, then your point is moot anyway and we may as well go home. > > I tried to merge hisi-smmu driver into arm-smmu.c, but it looks impossible. > > The biggest problem is that too many registers are diffrent: the base address, > > the field definition, or present only on one side. And if I use #if, hisi-smmu > > and arm-smmu can not coexist in one binary file. Almost need 20 #if. I don't think #ifdefs are necessarily the right way to go. There are IMPDEF parts of a compliant SMMU (e.g. power management), so having a new compatible string and a set of internal ops which can be overridden by a particular implementation wouldn't be the end of the world and would be far less invasive. The problem you have is that your SMMU isn't architecturally compliant, so you'll end up having to restructure parts of the driver so you can add internal hooks for operations that aren't actually IMPDEF. That's the part I'm worried about and we will have to draw the line somewhere, especially when other people are trying to work with this code for compliant implementations. So, NAK to your current approach. You need to try something like I said above and, if you want to spit the file up, start with the page-table code. Will ^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [PATCH v3 00/13] Add support for Hisilicon SMMU architecture 2014-07-21 9:39 ` [PATCH v3 00/13] Add support for Hisilicon SMMU architecture Will Deacon @ 2014-07-22 3:54 ` leizhen 0 siblings, 0 replies; 2+ messages in thread From: leizhen @ 2014-07-22 3:54 UTC (permalink / raw) To: Will Deacon; +Cc: huxinwei@huawei.com, iommu, Lizefan, linux-arm-kernel On 2014/7/21 17:39, Will Deacon wrote: > [re-adding the lists] > > On Mon, Jul 21, 2014 at 02:51:48AM +0100, leizhen wrote: >> Hi, Will > > Hello, > >> I have sent this patch a week ago. I saw that you and Mitchel Humpherys had >> sent some patches which will impact my code. I list below: >> iommu/arm-smmu: remove support for chained SMMUs >> iommu/arm-smmu: fix some checkpatch issues > > There's also all of the development work I'm doing for VFIO and the page-table > rework that Varun's working on, so these patches are going to be difficult > to merge if we ever get to that point. I think Mitchel and Olav are also > busy trying to get their SMMU supported by the driver. Given that they've > actually bothered to follow the architecture, I don't see why they should > be messed about by your patches causing conflicts all over the place. > >> I can ajust my patch because of above. I known this patch is not follow what >> you suggested before(fit into arm-smmu.c). But I found when we need to support >> ARM SMMUv3, maybe we should do like this. I really want to know whether or not >> you agree the software framework in this patch? Or what do you think about >> SMMUv3? > > The only code I foresee sharing between SMMUv2 and SMMUv3 is the page-table > creation code. The two architectures are significantly different, so I don't > think your split really helps us there. For example, you've left the probing > code in smmu-base.c. Of course, if HiSilicon follow the SMMUv3 spec as well > they did with SMMUv2, then your point is moot anyway and we may as well go > home. > >>> I tried to merge hisi-smmu driver into arm-smmu.c, but it looks impossible. >>> The biggest problem is that too many registers are diffrent: the base address, >>> the field definition, or present only on one side. And if I use #if, hisi-smmu >>> and arm-smmu can not coexist in one binary file. Almost need 20 #if. > > I don't think #ifdefs are necessarily the right way to go. There are IMPDEF > parts of a compliant SMMU (e.g. power management), so having a new compatible > string and a set of internal ops which can be overridden by a particular > implementation wouldn't be the end of the world and would be far less > invasive. > > The problem you have is that your SMMU isn't architecturally compliant, so > you'll end up having to restructure parts of the driver so you can add > internal hooks for operations that aren't actually IMPDEF. That's the part > I'm worried about and we will have to draw the line somewhere, especially > when other people are trying to work with this code for compliant > implementations. > > So, NAK to your current approach. You need to try something like I said > above and, if you want to spit the file up, start with the page-table code. > > Will > > . > OK. We have decided to discard current SMMU hardware design, and will completely follow the SMMUv3 specification. ^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2014-07-22 3:54 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <1404975186-12032-1-git-send-email-thunder.leizhen@huawei.com>
[not found] ` <53CC7234.4080703@huawei.com>
[not found] ` <53CC7234.4080703-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
2014-07-21 9:39 ` [PATCH v3 00/13] Add support for Hisilicon SMMU architecture Will Deacon
2014-07-22 3:54 ` leizhen
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).