xen/drivers/passthrough/arm/smmu.c | 162 ++++++++++++++++++++++++---------- xen/drivers/passthrough/device_tree.c | 24 ++--- 2 files changed, 123 insertions(+), 63 deletions(-)
Hi all,
This series introduces support for the generic SMMU bindings to
xen/drivers/passthrough/arm/smmu.c.
The last version of the series was
https://marc.info/?l=xen-devel&m=159539053406643
I realize that it is late for 4.15 -- I think it is OK if this series
goes in afterwards.
Cheers,
Stefano
Brian Woods (3):
arm,smmu: switch to using iommu_fwspec functions
arm,smmu: restructure code in preparation to new bindings support
arm,smmu: add support for generic DT bindings. Implement add_device and dt_xlate.
xen/drivers/passthrough/arm/smmu.c | 162 ++++++++++++++++++++++++----------
xen/drivers/passthrough/device_tree.c | 24 ++---
2 files changed, 123 insertions(+), 63 deletions(-)
Hello Stefano, > On 26 Jan 2021, at 10:58 pm, Stefano Stabellini <sstabellini@kernel.org> wrote: > > Hi all, > > This series introduces support for the generic SMMU bindings to > xen/drivers/passthrough/arm/smmu.c. > > The last version of the series was > https://marc.info/?l=xen-devel&m=159539053406643 > > I realize that it is late for 4.15 -- I think it is OK if this series > goes in afterwards. I tested the series on the Juno board it is woking fine. I found one issue in SMMU driver while testing this series that is not related to this series but already existing issue in SMMU driver. If there are more than one device behind SMMU and they share the same Stream-Id, SMMU driver is creating the new SMR entry without checking the already configured SMR entry if SMR entry correspond to stream-id is already configured. Because of this I observed the stream match conflicts on Juno board. (XEN) smmu: /iommu@7fb30000: Unexpected global fault, this could be serious (XEN) smmu: /iommu@7fb30000: GFSR 0x00000004, GFSYNR0 0x00000006, GFSYNR1 0x00000000, GFSYNR2 0x00000000 Below two patches is required to be ported to Xen to fix the issue from Linux driver. https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/drivers/iommu/arm-smmu.c?h=linux-5.8.y&id=1f3d5ca43019bff1105838712d55be087d93c0da https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/drivers/iommu/arm-smmu.c?h=linux-5.8.y&id=21174240e4f4439bb8ed6c116cdbdc03eba2126e Regards, Rahul > > Cheers, > > Stefano > > > Brian Woods (3): > arm,smmu: switch to using iommu_fwspec functions > arm,smmu: restructure code in preparation to new bindings support > arm,smmu: add support for generic DT bindings. Implement add_device and dt_xlate. > > xen/drivers/passthrough/arm/smmu.c | 162 ++++++++++++++++++++++++---------- > xen/drivers/passthrough/device_tree.c | 24 ++--- > 2 files changed, 123 insertions(+), 63 deletions(-)
On Tue, 2 Feb 2021, Rahul Singh wrote: > Hello Stefano, > > > On 26 Jan 2021, at 10:58 pm, Stefano Stabellini <sstabellini@kernel.org> wrote: > > > > Hi all, > > > > This series introduces support for the generic SMMU bindings to > > xen/drivers/passthrough/arm/smmu.c. > > > > The last version of the series was > > https://marc.info/?l=xen-devel&m=159539053406643 > > > > I realize that it is late for 4.15 -- I think it is OK if this series > > goes in afterwards. > > I tested the series on the Juno board it is woking fine. > I found one issue in SMMU driver while testing this series that is not related to this series but already existing issue in SMMU driver. > > If there are more than one device behind SMMU and they share the same Stream-Id, SMMU driver is creating the new SMR entry without checking the already configured SMR entry if SMR entry correspond to stream-id is already configured. Because of this I observed the stream match conflicts on Juno board. > > (XEN) smmu: /iommu@7fb30000: Unexpected global fault, this could be serious > (XEN) smmu: /iommu@7fb30000: GFSR 0x00000004, GFSYNR0 0x00000006, GFSYNR1 0x00000000, GFSYNR2 0x00000000 > > > Below two patches is required to be ported to Xen to fix the issue from Linux driver. > > https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/drivers/iommu/arm-smmu.c?h=linux-5.8.y&id=1f3d5ca43019bff1105838712d55be087d93c0da > https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/drivers/iommu/arm-smmu.c?h=linux-5.8.y&id=21174240e4f4439bb8ed6c116cdbdc03eba2126e Good catch and thanks for the pointers! Do you have any interest in backporting these two patches or should I put them on my TODO list? Unrelated to who does the job, we should discuss if it makes sense to try to fix the bug for 4.15. The patches don't seem trivial so I am tempted to say that it might be best to leave the bug unfixed for 4.15 and fix it later.
Hi, On 02/02/2021 17:44, Stefano Stabellini wrote: > On Tue, 2 Feb 2021, Rahul Singh wrote: >> Hello Stefano, >> >>> On 26 Jan 2021, at 10:58 pm, Stefano Stabellini <sstabellini@kernel.org> wrote: >>> >>> Hi all, >>> >>> This series introduces support for the generic SMMU bindings to >>> xen/drivers/passthrough/arm/smmu.c. >>> >>> The last version of the series was >>> https://marc.info/?l=xen-devel&m=159539053406643 >>> >>> I realize that it is late for 4.15 -- I think it is OK if this series >>> goes in afterwards. >> >> I tested the series on the Juno board it is woking fine. >> I found one issue in SMMU driver while testing this series that is not related to this series but already existing issue in SMMU driver. >> >> If there are more than one device behind SMMU and they share the same Stream-Id, SMMU driver is creating the new SMR entry without checking the already configured SMR entry if SMR entry correspond to stream-id is already configured. Because of this I observed the stream match conflicts on Juno board. >> >> (XEN) smmu: /iommu@7fb30000: Unexpected global fault, this could be serious >> (XEN) smmu: /iommu@7fb30000: GFSR 0x00000004, GFSYNR0 0x00000006, GFSYNR1 0x00000000, GFSYNR2 0x00000000 >> >> >> Below two patches is required to be ported to Xen to fix the issue from Linux driver. >> >> https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/drivers/iommu/arm-smmu.c?h=linux-5.8.y&id=1f3d5ca43019bff1105838712d55be087d93c0da >> https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/drivers/iommu/arm-smmu.c?h=linux-5.8.y&id=21174240e4f4439bb8ed6c116cdbdc03eba2126e > > > Good catch and thanks for the pointers! Do you have any interest in > backporting these two patches or should I put them on my TODO list? > > Unrelated to who does the job, we should discuss if it makes sense to > try to fix the bug for 4.15. The patches don't seem trivial so I am > tempted to say that it might be best to leave the bug unfixed for 4.15 > and fix it later. SMMU support on Juno is not that interesting because IIRC the stream-ID is the same for all the devices. So it is all or nothing passthrough. For other HW, this may be a useful feature. Yet we would need a way to group the devices for passthrough. In this context, I would consider it more a feature than a bug because the SMMU driver never remotely work on such HW. Cheers, -- Julien Grall
On Tue, 2 Feb 2021, Julien Grall wrote: > On 02/02/2021 17:44, Stefano Stabellini wrote: > > On Tue, 2 Feb 2021, Rahul Singh wrote: > > > Hello Stefano, > > > > > > > On 26 Jan 2021, at 10:58 pm, Stefano Stabellini <sstabellini@kernel.org> > > > > wrote: > > > > > > > > Hi all, > > > > > > > > This series introduces support for the generic SMMU bindings to > > > > xen/drivers/passthrough/arm/smmu.c. > > > > > > > > The last version of the series was > > > > https://marc.info/?l=xen-devel&m=159539053406643 > > > > > > > > I realize that it is late for 4.15 -- I think it is OK if this series > > > > goes in afterwards. > > > > > > I tested the series on the Juno board it is woking fine. > > > I found one issue in SMMU driver while testing this series that is not > > > related to this series but already existing issue in SMMU driver. > > > > > > If there are more than one device behind SMMU and they share the same > > > Stream-Id, SMMU driver is creating the new SMR entry without checking the > > > already configured SMR entry if SMR entry correspond to stream-id is > > > already configured. Because of this I observed the stream match conflicts > > > on Juno board. > > > > > > (XEN) smmu: /iommu@7fb30000: Unexpected global fault, this could be > > > serious > > > (XEN) smmu: /iommu@7fb30000: GFSR 0x00000004, GFSYNR0 0x00000006, > > > GFSYNR1 0x00000000, GFSYNR2 0x00000000 > > > > > > > > > Below two patches is required to be ported to Xen to fix the issue from > > > Linux driver. > > > > > > https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/drivers/iommu/arm-smmu.c?h=linux-5.8.y&id=1f3d5ca43019bff1105838712d55be087d93c0da > > > https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/drivers/iommu/arm-smmu.c?h=linux-5.8.y&id=21174240e4f4439bb8ed6c116cdbdc03eba2126e > > > > > > Good catch and thanks for the pointers! Do you have any interest in > > backporting these two patches or should I put them on my TODO list? > > > > Unrelated to who does the job, we should discuss if it makes sense to > > try to fix the bug for 4.15. The patches don't seem trivial so I am > > tempted to say that it might be best to leave the bug unfixed for 4.15 > > and fix it later. > > SMMU support on Juno is not that interesting because IIRC the stream-ID is the > same for all the devices. So it is all or nothing passthrough. > > For other HW, this may be a useful feature. Yet we would need a way to group > the devices for passthrough. > > In this context, I would consider it more a feature than a bug because the > SMMU driver never remotely work on such HW. I see. To be honest I wasn't thinking of Juno (I wasn't aware of its limitations) but of potential genuine situations where stream-ids are the same for 2 devices. I know it can happen with PCI devices for instance, although I am aware we don't have PCI passthrough yet. I don't know if it is possible for it to happen with non-PCI devices but I wouldn't be surprised if it can.
Hi Stefano, On 02/02/2021 18:27, Stefano Stabellini wrote: > On Tue, 2 Feb 2021, Julien Grall wrote: >> On 02/02/2021 17:44, Stefano Stabellini wrote: >>> On Tue, 2 Feb 2021, Rahul Singh wrote: >>>> Hello Stefano, >>>> >>>>> On 26 Jan 2021, at 10:58 pm, Stefano Stabellini <sstabellini@kernel.org> >>>>> wrote: >>>>> >>>>> Hi all, >>>>> >>>>> This series introduces support for the generic SMMU bindings to >>>>> xen/drivers/passthrough/arm/smmu.c. >>>>> >>>>> The last version of the series was >>>>> https://marc.info/?l=xen-devel&m=159539053406643 >>>>> >>>>> I realize that it is late for 4.15 -- I think it is OK if this series >>>>> goes in afterwards. >>>> >>>> I tested the series on the Juno board it is woking fine. >>>> I found one issue in SMMU driver while testing this series that is not >>>> related to this series but already existing issue in SMMU driver. >>>> >>>> If there are more than one device behind SMMU and they share the same >>>> Stream-Id, SMMU driver is creating the new SMR entry without checking the >>>> already configured SMR entry if SMR entry correspond to stream-id is >>>> already configured. Because of this I observed the stream match conflicts >>>> on Juno board. >>>> >>>> (XEN) smmu: /iommu@7fb30000: Unexpected global fault, this could be >>>> serious >>>> (XEN) smmu: /iommu@7fb30000: GFSR 0x00000004, GFSYNR0 0x00000006, >>>> GFSYNR1 0x00000000, GFSYNR2 0x00000000 >>>> >>>> >>>> Below two patches is required to be ported to Xen to fix the issue from >>>> Linux driver. >>>> >>>> https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/drivers/iommu/arm-smmu.c?h=linux-5.8.y&id=1f3d5ca43019bff1105838712d55be087d93c0da >>>> https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/drivers/iommu/arm-smmu.c?h=linux-5.8.y&id=21174240e4f4439bb8ed6c116cdbdc03eba2126e >>> >>> >>> Good catch and thanks for the pointers! Do you have any interest in >>> backporting these two patches or should I put them on my TODO list? >>> >>> Unrelated to who does the job, we should discuss if it makes sense to >>> try to fix the bug for 4.15. The patches don't seem trivial so I am >>> tempted to say that it might be best to leave the bug unfixed for 4.15 >>> and fix it later. >> >> SMMU support on Juno is not that interesting because IIRC the stream-ID is the >> same for all the devices. So it is all or nothing passthrough. >> >> For other HW, this may be a useful feature. Yet we would need a way to group >> the devices for passthrough. >> >> In this context, I would consider it more a feature than a bug because the >> SMMU driver never remotely work on such HW. > > I see. To be honest I wasn't thinking of Juno (I wasn't aware of its > limitations) but of potential genuine situations where stream-ids are > the same for 2 devices. I know it can happen with PCI devices for > instance, although I am aware we don't have PCI passthrough yet. I don't > know if it is possible for it to happen with non-PCI devices but I > wouldn't be surprised if it can. I merely pointed out Juno because this is where the discussion started. Although, my conclusion wasn't solely based on this system nor PCI devices. It was based on the fact that this could never have worked with the current SMMU driver. So this is not a regression and more an improvement of the driver to support passthrough for devices using the same stream-ID. At this stage of the release, I would only consider trivial improvement to be merged. Cheers, -- Julien Grall
Hello Stefano, > On 2 Feb 2021, at 5:44 pm, Stefano Stabellini <sstabellini@kernel.org> wrote: > > On Tue, 2 Feb 2021, Rahul Singh wrote: >> Hello Stefano, >> >>> On 26 Jan 2021, at 10:58 pm, Stefano Stabellini <sstabellini@kernel.org> wrote: >>> >>> Hi all, >>> >>> This series introduces support for the generic SMMU bindings to >>> xen/drivers/passthrough/arm/smmu.c. >>> >>> The last version of the series was >>> https://marc.info/?l=xen-devel&m=159539053406643 >>> >>> I realize that it is late for 4.15 -- I think it is OK if this series >>> goes in afterwards. >> >> I tested the series on the Juno board it is woking fine. >> I found one issue in SMMU driver while testing this series that is not related to this series but already existing issue in SMMU driver. >> >> If there are more than one device behind SMMU and they share the same Stream-Id, SMMU driver is creating the new SMR entry without checking the already configured SMR entry if SMR entry correspond to stream-id is already configured. Because of this I observed the stream match conflicts on Juno board. >> >> (XEN) smmu: /iommu@7fb30000: Unexpected global fault, this could be serious >> (XEN) smmu: /iommu@7fb30000: GFSR 0x00000004, GFSYNR0 0x00000006, GFSYNR1 0x00000000, GFSYNR2 0x00000000 >> >> >> Below two patches is required to be ported to Xen to fix the issue from Linux driver. >> >> https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/drivers/iommu/arm-smmu.c?h=linux-5.8.y&id=1f3d5ca43019bff1105838712d55be087d93c0da >> https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/drivers/iommu/arm-smmu.c?h=linux-5.8.y&id=21174240e4f4439bb8ed6c116cdbdc03eba2126e > > > Good catch and thanks for the pointers! Do you have any interest in > backporting these two patches or should I put them on my TODO list? Yes I am happy to backport these patches to XEN. I will send the patch for review once 4.15 release branch out from master. Regards, Rahul > > Unrelated to who does the job, we should discuss if it makes sense to > try to fix the bug for 4.15. The patches don't seem trivial so I am > tempted to say that it might be best to leave the bug unfixed for 4.15 > and fix it later.
On 26/01/2021 22:58, Stefano Stabellini wrote: > Hi all, Hi Stefano, > This series introduces support for the generic SMMU bindings to > xen/drivers/passthrough/arm/smmu.c. > > The last version of the series was > https://marc.info/?l=xen-devel&m=159539053406643 Some changes in the SMMU drivers went in recently. I believe this touched similar area to this series. Would you be able to check that this series still work as intented? Cheers, -- Julien Grall
On Mon, 5 Apr 2021, Julien Grall wrote: > On 26/01/2021 22:58, Stefano Stabellini wrote: > > Hi all, > > Hi Stefano, > > > This series introduces support for the generic SMMU bindings to > > xen/drivers/passthrough/arm/smmu.c. > > > > The last version of the series was > > https://marc.info/?l=xen-devel&m=159539053406643 > Some changes in the SMMU drivers went in recently. I believe this touched > similar area to this series. Would you be able to check that this series still > work as intented? Thanks for the heads up, no, unfortunately they don't work :-( They badly clash. I did the forward port of the three patches but they fail at runtime in my tests. I ran out of time to figure out what is the problem, and I'll have to pick it up at some point in the future (it might not be any time soon unfortunately). Rahul, if you have any ideas about what the problem is please let me know. This is the branch with the forward port: http://xenbits.xenproject.org/git-http/people/sstabellini/xen-unstable.git smmu-generic
Hi Stefano, > On 7 Apr 2021, at 12:59 am, Stefano Stabellini <sstabellini@kernel.org> wrote: > > On Mon, 5 Apr 2021, Julien Grall wrote: >> On 26/01/2021 22:58, Stefano Stabellini wrote: >>> Hi all, >> >> Hi Stefano, >> >>> This series introduces support for the generic SMMU bindings to >>> xen/drivers/passthrough/arm/smmu.c. >>> >>> The last version of the series was >>> https://marc.info/?l=xen-devel&m=159539053406643 >> Some changes in the SMMU drivers went in recently. I believe this touched >> similar area to this series. Would you be able to check that this series still >> work as intented? > > Thanks for the heads up, no, unfortunately they don't work :-( > > They badly clash. I did the forward port of the three patches but they > fail at runtime in my tests. I ran out of time to figure out what is the > problem, and I'll have to pick it up at some point in the future (it > might not be any time soon unfortunately). > > Rahul, if you have any ideas about what the problem is please let me > know. This is the branch with the forward port: Let me check and come back to you if I found out anything regarding the same. Regards, Rahul > > http://xenbits.xenproject.org/git-http/people/sstabellini/xen-unstable.git smmu-generic
On Tue, 6 Apr 2021, Stefano Stabellini wrote: > On Mon, 5 Apr 2021, Julien Grall wrote: > > On 26/01/2021 22:58, Stefano Stabellini wrote: > > > Hi all, > > > > Hi Stefano, > > > > > This series introduces support for the generic SMMU bindings to > > > xen/drivers/passthrough/arm/smmu.c. > > > > > > The last version of the series was > > > https://marc.info/?l=xen-devel&m=159539053406643 > > Some changes in the SMMU drivers went in recently. I believe this touched > > similar area to this series. Would you be able to check that this series still > > work as intented? > > Thanks for the heads up, no, unfortunately they don't work :-( > > They badly clash. I did the forward port of the three patches but they > fail at runtime in my tests. I ran out of time to figure out what is the > problem, and I'll have to pick it up at some point in the future (it > might not be any time soon unfortunately). > > Rahul, if you have any ideas about what the problem is please let me > know. This is the branch with the forward port: > > http://xenbits.xenproject.org/git-http/people/sstabellini/xen-unstable.git smmu-generic I did some more investigation and spotted a minor error in my forward port. This an updated branch based on staging: http://xenbits.xenproject.org/git-http/people/sstabellini/xen-unstable.git smmu-generic-2 However, the real issue is that Rahul's patches break SMMU support on ZynqMP even without my changes. I ran a bisection and found out that patch #2 is the culprit: 5ee3fa0b21ea xen/arm: smmuv1: Consolidate stream map entry state It causes the abort appended below. The problem doesn't seem particularly ZynqMP specific. Rahul, can you reproduce it on your side? (XEN) smmu: /amba/smmu@fd800000: d0: p2maddr 0x000000087bfa2000 (XEN) Data Abort Trap. Syndrome=0x5 (XEN) Walking Hypervisor VA 0x114ebfff8 on CPU0 via TTBR 0x0000000000f38000 (XEN) 0TH[0x0] = 0x0000000000f3bf7f (XEN) 1ST[0x4] = 0x0000000000000000 (XEN) CPU0: Unexpected Trap: Data Abort (XEN) ----[ Xen-4.14.0 arm64 debug=y Not tainted ]---- (XEN) CPU: 0 (XEN) PC: 000000000024a77c smmu.c#find_smmu_master+0x8/0x3c (XEN) LR: 000000000024a8a4 (XEN) SP: 00000000002ff1f0 (XEN) CPSR: 80000249 MODE:64-bit EL2h (Hypervisor, handler) (XEN) X0: 0000000114ec0000 X1: 00008000fbfc7478 X2: 00008000fbfc71e0 (XEN) X3: 00000000002af840 X4: 0000000000000000 X5: 0000000000000001 (XEN) X6: 0000000000000000 X7: 0000000000000000 X8: 00008000fbf8b9e0 (XEN) X9: 0000000000000004 X10: 0101010101010101 X11: 0000000000000020 (XEN) X12: 0000000000000018 X13: ff00000000000000 X14: 0400000084000000 (XEN) X15: 0000000000000000 X16: 00000000002b1000 X17: 00000000002b1000 (XEN) X18: 00000000002b2000 X19: 00008000fbffcb70 X20: 00000000002af848 (XEN) X21: 00008000fbfc7478 X22: 00008000fbfc74d8 X23: 00008000fbfc7508 (XEN) X24: 0000000000000000 X25: 0000000000000001 X26: 00008000fbfa7c20 (XEN) X27: 0000000000000000 X28: 0000000000000000 FP: 00000000002ff1f0 (XEN) (XEN) VTCR_EL2: 80023558 (XEN) VTTBR_EL2: 000000087bf54000 (XEN) (XEN) SCTLR_EL2: 30cd183d (XEN) HCR_EL2: 000000000000003a (XEN) TTBR0_EL2: 0000000000f38000 (XEN) (XEN) ESR_EL2: 96000005 (XEN) HPFAR_EL2: 0000000000220000 (XEN) FAR_EL2: 0000000114ebfff8 (XEN) (XEN) Xen stack trace from sp=00000000002ff1f0: (XEN) 00000000002ff220 000000000024ae80 00008000fbfa5000 00008000fbfc74e8 (XEN) 00008000fbfa5000 0000000800000001 00000000002ff2a0 000000000024c6e8 (XEN) 00008000fbfa5000 00000000fffffff0 00008000fbfc7478 00008000fbfc74d8 (XEN) 00008000fbfc7508 0000000000000000 0000000000000001 0000000000000001 (XEN) 0000000000000000 0000000000000000 00000000002ff2a0 000000000024c6b8 (XEN) 00008000fbfa5000 00000000002ff550 00000000002ff2d0 00000000002c6274 (XEN) 00008000fbfc7478 00000000002ff550 00008000fbfa5000 0000000000000005 (XEN) 00000000002ff390 00000000002c6704 00008000fbfc3ce8 00000000002ff550 (XEN) 00008000fbfa5000 0000000000000005 00008000fbfc7478 0000000000000000 (XEN) 00008000fbff1100 0000000000000000 0000000000000000 0000000000000000 (XEN) 00000000002d28e8 00000000fbf78090 00000000002d28d8 00000000002d1b80 (XEN) 00000000002ff380 00000000ff0b0000 00008000fbfc3ce8 0000000000001000 (XEN) 00008000fbfa5000 00008000fbfa5000 0000000000000005 00000000002c6600 (XEN) 00000000002ff450 00000000002c6704 00008000fbfc0000 00000000002ff550 (XEN) 00008000fbfa5000 0000000000000005 00008000fbfc3ce8 0000000000000000 (XEN) 00008000fbff00a8 0000000000000013 0000000000000000 0000000000000000 (XEN) 00000000002d28e8 00000000fbf78090 00000000002d28d8 00000000002d1b80 (XEN) 00000000002ff440 00000000ff990000 00008000fbfc0000 0000000000001000 (XEN) 00008000fbfa5000 00008000fbfa5000 0000000000000005 00000000002c6600 (XEN) 00000000002ff510 00000000002c6f78 0000000000008090 0000000000e00000 (XEN) 00000000002d2ae8 00008000fbfa5000 000000000000000f 0000000000000004 (XEN) 00000000002e05e0 0000000000000000 0000000880000000 0000000000000002 (XEN) 00000000002d28e8 000000000022d1e4 00000000002d28d8 00000000002d1b80 (XEN) 00000000002ff500 00000000002b7da0 0000000000008090 0000000000e00000 (XEN) 00000000002d2ae8 00008000fbfa5000 0000000000000005 00000000002c6f60 (XEN) 00000000002ffdf0 00000000002cb1fc 00008000fbfa5000 00000000002b0600 (XEN) 0000000000340430 0000000000000004 00000000002a3810 0000000000000000 (XEN) 0000000000000001 00008000fbfa5000 00008000fbf70000 0000000000000000 (XEN) 0000000000000001 0000000020000000 0000000040000000 0000000000000000 (XEN) 0000000000000000 0000000000000000 0000000000000000 0000000000000000 (XEN) 0000000000000000 0000000000000000 0000000000000000 0000000000000000 (XEN) 0000000000000000 0000000000000000 0000000000000000 0000000000000000 (XEN) 0000000000000000 0000000000000000 0000000000000000 0000000000000000 (XEN) 0000000000000000 0000000000000000 0000000000000000 0000000000000000 (XEN) 0000000000000000 0000000000000000 0000000000000000 0000000000000000 (XEN) 0000000000000000 0000000000000000 0000000000000000 0000000000000000 (XEN) 0000000000000000 0000000000000000 0000000000000000 0000000000000000 (XEN) 0000000000000000 0000000000000000 0000000000000000 0000000000000000 (XEN) 0000000000000000 0000000000000000 0000000000000000 0000000000000000 (XEN) 0000000000000000 0000000000000000 0000000000000000 0000000000000000 (XEN) Xen call trace: (XEN) [<000000000024a77c>] smmu.c#find_smmu_master+0x8/0x3c (PC) (XEN) [<000000000024a8a4>] smmu.c#find_smmu_for_device+0x48/0x94 (LR) (XEN) [<000000000024ae80>] smmu.c#arm_smmu_assign_dev+0x58/0xb48 (XEN) [<000000000024c6e8>] iommu_assign_dt_device+0x64/0xc0 (XEN) [<00000000002c6274>] domain_build.c#handle_node+0x310/0x9ec (XEN) [<00000000002c6704>] domain_build.c#handle_node+0x7a0/0x9ec (XEN) [<00000000002c6704>] domain_build.c#handle_node+0x7a0/0x9ec (XEN) [<00000000002c6f78>] construct_dom0+0x410/0x4bc (XEN) [<00000000002cb1fc>] start_xen+0xb9c/0xca4 (XEN) [<00000000002001a0>] arm64/head.o#primary_switched+0xc/0x1c
Hi Stefano,
> On 10 Apr 2021, at 1:27 am, Stefano Stabellini <sstabellini@kernel.org> wrote:
>
> On Tue, 6 Apr 2021, Stefano Stabellini wrote:
>> On Mon, 5 Apr 2021, Julien Grall wrote:
>>> On 26/01/2021 22:58, Stefano Stabellini wrote:
>>>> Hi all,
>>>
>>> Hi Stefano,
>>>
>>>> This series introduces support for the generic SMMU bindings to
>>>> xen/drivers/passthrough/arm/smmu.c.
>>>>
>>>> The last version of the series was
>>>> https://marc.info/?l=xen-devel&m=159539053406643
>>> Some changes in the SMMU drivers went in recently. I believe this touched
>>> similar area to this series. Would you be able to check that this series still
>>> work as intented?
>>
>> Thanks for the heads up, no, unfortunately they don't work :-(
>>
>> They badly clash. I did the forward port of the three patches but they
>> fail at runtime in my tests. I ran out of time to figure out what is the
>> problem, and I'll have to pick it up at some point in the future (it
>> might not be any time soon unfortunately).
>>
>> Rahul, if you have any ideas about what the problem is please let me
>> know. This is the branch with the forward port:
>>
>> http://xenbits.xenproject.org/git-http/people/sstabellini/xen-unstable.git smmu-generic
>
> I did some more investigation and spotted a minor error in my forward
> port. This an updated branch based on staging:
>
> http://xenbits.xenproject.org/git-http/people/sstabellini/xen-unstable.git smmu-generic-2
>
> However, the real issue is that Rahul's patches break SMMU support on
> ZynqMP even without my changes. I ran a bisection and found out that
> patch #2 is the culprit:
>
> 5ee3fa0b21ea xen/arm: smmuv1: Consolidate stream map entry state
>
> It causes the abort appended below. The problem doesn't seem
> particularly ZynqMP specific. Rahul, can you reproduce it on your side?
Yes. I can reproduce the issue on Xilinx QEMU as we don’t have access to physical ZynqMP and found out that
associating an iommu group pointer with the S2CR causing the issue.
Associating the group pointer with S2CR is part of the patch "xen/arm: smmuv1: Intelligent SMR allocation”.
I just revert that part of the code from the patch and it works fine for me. Please find the attached patch for the same.
As per your analysis "5ee3fa0b21ea xen/arm: smmuv1: Consolidate stream map entry state” is causing the issue but what I found out that
"xen/arm: smmuv1: Intelligent SMR allocation” is causing the issue.
Can you please test it on the physical device and let me know if it works for you also to make sure we both observing the same issue.
Regards,
Rahul
>
>
> (XEN) smmu: /amba/smmu@fd800000: d0: p2maddr 0x000000087bfa2000
> (XEN) Data Abort Trap. Syndrome=0x5
> (XEN) Walking Hypervisor VA 0x114ebfff8 on CPU0 via TTBR 0x0000000000f38000
> (XEN) 0TH[0x0] = 0x0000000000f3bf7f
> (XEN) 1ST[0x4] = 0x0000000000000000
> (XEN) CPU0: Unexpected Trap: Data Abort
> (XEN) ----[ Xen-4.14.0 arm64 debug=y Not tainted ]----
> (XEN) CPU: 0
> (XEN) PC: 000000000024a77c smmu.c#find_smmu_master+0x8/0x3c
> (XEN) LR: 000000000024a8a4
> (XEN) SP: 00000000002ff1f0
> (XEN) CPSR: 80000249 MODE:64-bit EL2h (Hypervisor, handler)
> (XEN) X0: 0000000114ec0000 X1: 00008000fbfc7478 X2: 00008000fbfc71e0
> (XEN) X3: 00000000002af840 X4: 0000000000000000 X5: 0000000000000001
> (XEN) X6: 0000000000000000 X7: 0000000000000000 X8: 00008000fbf8b9e0
> (XEN) X9: 0000000000000004 X10: 0101010101010101 X11: 0000000000000020
> (XEN) X12: 0000000000000018 X13: ff00000000000000 X14: 0400000084000000
> (XEN) X15: 0000000000000000 X16: 00000000002b1000 X17: 00000000002b1000
> (XEN) X18: 00000000002b2000 X19: 00008000fbffcb70 X20: 00000000002af848
> (XEN) X21: 00008000fbfc7478 X22: 00008000fbfc74d8 X23: 00008000fbfc7508
> (XEN) X24: 0000000000000000 X25: 0000000000000001 X26: 00008000fbfa7c20
> (XEN) X27: 0000000000000000 X28: 0000000000000000 FP: 00000000002ff1f0
> (XEN)
> (XEN) VTCR_EL2: 80023558
> (XEN) VTTBR_EL2: 000000087bf54000
> (XEN)
> (XEN) SCTLR_EL2: 30cd183d
> (XEN) HCR_EL2: 000000000000003a
> (XEN) TTBR0_EL2: 0000000000f38000
> (XEN)
> (XEN) ESR_EL2: 96000005
> (XEN) HPFAR_EL2: 0000000000220000
> (XEN) FAR_EL2: 0000000114ebfff8
> (XEN)
> (XEN) Xen stack trace from sp=00000000002ff1f0:
> (XEN) 00000000002ff220 000000000024ae80 00008000fbfa5000 00008000fbfc74e8
> (XEN) 00008000fbfa5000 0000000800000001 00000000002ff2a0 000000000024c6e8
> (XEN) 00008000fbfa5000 00000000fffffff0 00008000fbfc7478 00008000fbfc74d8
> (XEN) 00008000fbfc7508 0000000000000000 0000000000000001 0000000000000001
> (XEN) 0000000000000000 0000000000000000 00000000002ff2a0 000000000024c6b8
> (XEN) 00008000fbfa5000 00000000002ff550 00000000002ff2d0 00000000002c6274
> (XEN) 00008000fbfc7478 00000000002ff550 00008000fbfa5000 0000000000000005
> (XEN) 00000000002ff390 00000000002c6704 00008000fbfc3ce8 00000000002ff550
> (XEN) 00008000fbfa5000 0000000000000005 00008000fbfc7478 0000000000000000
> (XEN) 00008000fbff1100 0000000000000000 0000000000000000 0000000000000000
> (XEN) 00000000002d28e8 00000000fbf78090 00000000002d28d8 00000000002d1b80
> (XEN) 00000000002ff380 00000000ff0b0000 00008000fbfc3ce8 0000000000001000
> (XEN) 00008000fbfa5000 00008000fbfa5000 0000000000000005 00000000002c6600
> (XEN) 00000000002ff450 00000000002c6704 00008000fbfc0000 00000000002ff550
> (XEN) 00008000fbfa5000 0000000000000005 00008000fbfc3ce8 0000000000000000
> (XEN) 00008000fbff00a8 0000000000000013 0000000000000000 0000000000000000
> (XEN) 00000000002d28e8 00000000fbf78090 00000000002d28d8 00000000002d1b80
> (XEN) 00000000002ff440 00000000ff990000 00008000fbfc0000 0000000000001000
> (XEN) 00008000fbfa5000 00008000fbfa5000 0000000000000005 00000000002c6600
> (XEN) 00000000002ff510 00000000002c6f78 0000000000008090 0000000000e00000
> (XEN) 00000000002d2ae8 00008000fbfa5000 000000000000000f 0000000000000004
> (XEN) 00000000002e05e0 0000000000000000 0000000880000000 0000000000000002
> (XEN) 00000000002d28e8 000000000022d1e4 00000000002d28d8 00000000002d1b80
> (XEN) 00000000002ff500 00000000002b7da0 0000000000008090 0000000000e00000
> (XEN) 00000000002d2ae8 00008000fbfa5000 0000000000000005 00000000002c6f60
> (XEN) 00000000002ffdf0 00000000002cb1fc 00008000fbfa5000 00000000002b0600
> (XEN) 0000000000340430 0000000000000004 00000000002a3810 0000000000000000
> (XEN) 0000000000000001 00008000fbfa5000 00008000fbf70000 0000000000000000
> (XEN) 0000000000000001 0000000020000000 0000000040000000 0000000000000000
> (XEN) 0000000000000000 0000000000000000 0000000000000000 0000000000000000
> (XEN) 0000000000000000 0000000000000000 0000000000000000 0000000000000000
> (XEN) 0000000000000000 0000000000000000 0000000000000000 0000000000000000
> (XEN) 0000000000000000 0000000000000000 0000000000000000 0000000000000000
> (XEN) 0000000000000000 0000000000000000 0000000000000000 0000000000000000
> (XEN) 0000000000000000 0000000000000000 0000000000000000 0000000000000000
> (XEN) 0000000000000000 0000000000000000 0000000000000000 0000000000000000
> (XEN) 0000000000000000 0000000000000000 0000000000000000 0000000000000000
> (XEN) 0000000000000000 0000000000000000 0000000000000000 0000000000000000
> (XEN) 0000000000000000 0000000000000000 0000000000000000 0000000000000000
> (XEN) 0000000000000000 0000000000000000 0000000000000000 0000000000000000
> (XEN) Xen call trace:
> (XEN) [<000000000024a77c>] smmu.c#find_smmu_master+0x8/0x3c (PC)
> (XEN) [<000000000024a8a4>] smmu.c#find_smmu_for_device+0x48/0x94 (LR)
> (XEN) [<000000000024ae80>] smmu.c#arm_smmu_assign_dev+0x58/0xb48
> (XEN) [<000000000024c6e8>] iommu_assign_dt_device+0x64/0xc0
> (XEN) [<00000000002c6274>] domain_build.c#handle_node+0x310/0x9ec
> (XEN) [<00000000002c6704>] domain_build.c#handle_node+0x7a0/0x9ec
> (XEN) [<00000000002c6704>] domain_build.c#handle_node+0x7a0/0x9ec
> (XEN) [<00000000002c6f78>] construct_dom0+0x410/0x4bc
> (XEN) [<00000000002cb1fc>] start_xen+0xb9c/0xca4
> (XEN) [<00000000002001a0>] arm64/head.o#primary_switched+0xc/0x1c
From 02278fa0e4abd41e5c5e253fe23b604a4f081105 Mon Sep 17 00:00:00 2001
Message-Id: <02278fa0e4abd41e5c5e253fe23b604a4f081105.1618222589.git.rahul.singh@arm.com>
From: Rahul Singh <rahul.singh@arm.com>
Date: Mon, 12 Apr 2021 09:50:05 +0100
Subject: [PATCH] xen/arm: smmuv1: Revert associating the group pointer with
the S2CR
Revert the code that associate the group pointer with the S2CR as this
causing an issue when SMMU device has more that one master device.
Signed-off-by: Rahul Singh <rahul.singh@arm.com>
---
xen/drivers/passthrough/arm/smmu.c | 44 +++---------------------------
1 file changed, 4 insertions(+), 40 deletions(-)
diff --git a/xen/drivers/passthrough/arm/smmu.c b/xen/drivers/passthrough/arm/smmu.c
index 20ac672e91..3456daa03f 100644
--- a/xen/drivers/passthrough/arm/smmu.c
+++ b/xen/drivers/passthrough/arm/smmu.c
@@ -597,7 +597,6 @@ enum arm_smmu_arch_version {
};
struct arm_smmu_s2cr {
- struct iommu_group *group;
int count;
enum arm_smmu_s2cr_type type;
enum arm_smmu_s2cr_privcfg privcfg;
@@ -1498,7 +1497,6 @@ static int arm_smmu_master_alloc_smes(struct device *dev)
struct arm_smmu_master_cfg *cfg = find_smmu_master_cfg(dev);
struct arm_smmu_device *smmu = cfg->smmu;
struct arm_smmu_smr *smrs = smmu->smrs;
- struct iommu_group *group;
int i, idx, ret;
spin_lock(&smmu->stream_map_lock);
@@ -1523,19 +1521,9 @@ static int arm_smmu_master_alloc_smes(struct device *dev)
cfg->smendx[i] = (s16)idx;
}
- group = iommu_group_get(dev);
- if (!group)
- group = ERR_PTR(-ENOMEM);
- if (IS_ERR(group)) {
- ret = PTR_ERR(group);
- goto out_err;
- }
- iommu_group_put(group);
-
/* It worked! Now, poke the actual hardware */
for_each_cfg_sme(cfg, i, idx) {
arm_smmu_write_sme(smmu, idx);
- smmu->s2crs[idx].group = group;
}
spin_unlock(&smmu->stream_map_lock);
@@ -1966,27 +1954,6 @@ static void __arm_smmu_release_pci_iommudata(void *data)
kfree(data);
}
-static struct iommu_group *arm_smmu_device_group(struct
- arm_smmu_master_cfg *cfg)
-{
- struct arm_smmu_device *smmu = cfg->smmu;
- struct iommu_group *group = NULL;
- int i, idx;
-
- for_each_cfg_sme(cfg, i, idx) {
- if (group && smmu->s2crs[idx].group &&
- group != smmu->s2crs[idx].group)
- return ERR_PTR(-EINVAL);
-
- group = smmu->s2crs[idx].group;
- }
-
- if (group)
- return group;
-
- return NULL;
-}
-
static int arm_smmu_add_device(struct device *dev)
{
struct arm_smmu_device *smmu;
@@ -2027,13 +1994,10 @@ static int arm_smmu_add_device(struct device *dev)
cfg->smmu = smmu;
}
- group = arm_smmu_device_group(cfg);
- if (!group) {
- group = iommu_group_alloc();
- if (IS_ERR(group)) {
- dev_err(dev, "Failed to allocate IOMMU group\n");
- return PTR_ERR(group);
- }
+ group = iommu_group_alloc();
+ if (IS_ERR(group)) {
+ dev_err(dev, "Failed to allocate IOMMU group\n");
+ return PTR_ERR(group);
}
iommu_group_set_iommudata(group, cfg, releasefn);
--
2.17.1
On Mon, 12 Apr 2021, Rahul Singh wrote: > Hi Stefano, > > > On 10 Apr 2021, at 1:27 am, Stefano Stabellini <sstabellini@kernel.org> wrote: > > > > On Tue, 6 Apr 2021, Stefano Stabellini wrote: > >> On Mon, 5 Apr 2021, Julien Grall wrote: > >>> On 26/01/2021 22:58, Stefano Stabellini wrote: > >>>> Hi all, > >>> > >>> Hi Stefano, > >>> > >>>> This series introduces support for the generic SMMU bindings to > >>>> xen/drivers/passthrough/arm/smmu.c. > >>>> > >>>> The last version of the series was > >>>> https://marc.info/?l=xen-devel&m=159539053406643 > >>> Some changes in the SMMU drivers went in recently. I believe this touched > >>> similar area to this series. Would you be able to check that this series still > >>> work as intented? > >> > >> Thanks for the heads up, no, unfortunately they don't work :-( > >> > >> They badly clash. I did the forward port of the three patches but they > >> fail at runtime in my tests. I ran out of time to figure out what is the > >> problem, and I'll have to pick it up at some point in the future (it > >> might not be any time soon unfortunately). > >> > >> Rahul, if you have any ideas about what the problem is please let me > >> know. This is the branch with the forward port: > >> > >> http://xenbits.xenproject.org/git-http/people/sstabellini/xen-unstable.git smmu-generic > > > > I did some more investigation and spotted a minor error in my forward > > port. This an updated branch based on staging: > > > > http://xenbits.xenproject.org/git-http/people/sstabellini/xen-unstable.git smmu-generic-2 > > > > However, the real issue is that Rahul's patches break SMMU support on > > ZynqMP even without my changes. I ran a bisection and found out that > > patch #2 is the culprit: > > > > 5ee3fa0b21ea xen/arm: smmuv1: Consolidate stream map entry state > > > > It causes the abort appended below. The problem doesn't seem > > particularly ZynqMP specific. Rahul, can you reproduce it on your side? > > Yes. I can reproduce the issue on Xilinx QEMU as we don’t have access to physical ZynqMP and found out that > associating an iommu group pointer with the S2CR causing the issue. > > Associating the group pointer with S2CR is part of the patch "xen/arm: smmuv1: Intelligent SMR allocation”. > > I just revert that part of the code from the patch and it works fine for me. Please find the attached patch for the same. > > As per your analysis "5ee3fa0b21ea xen/arm: smmuv1: Consolidate stream map entry state” is causing the issue but what I found out that > "xen/arm: smmuv1: Intelligent SMR allocation” is causing the issue. > Can you please test it on the physical device and let me know if it works for you also to make sure we both observing the same issue. Great! Yes, I can confirm that your patch fixed the issue, now I can boot staging on ZynqMP without errors and I can do device assignment too. Thank you so much! The other good news is that the three "Generic SMMU Bindings" patches work too on top of yours with the fix! Is the patch you submitted the valid fix for the problem? In other words, should we go ahead, review, and commit the patch you attached or do you want to send a different version of the patch for inclusion in Xen staging?
HI Stefano, > On 13 Apr 2021, at 1:27 am, Stefano Stabellini <sstabellini@kernel.org> wrote: > > On Mon, 12 Apr 2021, Rahul Singh wrote: >> Hi Stefano, >> >>> On 10 Apr 2021, at 1:27 am, Stefano Stabellini <sstabellini@kernel.org> wrote: >>> >>> On Tue, 6 Apr 2021, Stefano Stabellini wrote: >>>> On Mon, 5 Apr 2021, Julien Grall wrote: >>>>> On 26/01/2021 22:58, Stefano Stabellini wrote: >>>>>> Hi all, >>>>> >>>>> Hi Stefano, >>>>> >>>>>> This series introduces support for the generic SMMU bindings to >>>>>> xen/drivers/passthrough/arm/smmu.c. >>>>>> >>>>>> The last version of the series was >>>>>> https://marc.info/?l=xen-devel&m=159539053406643 >>>>> Some changes in the SMMU drivers went in recently. I believe this touched >>>>> similar area to this series. Would you be able to check that this series still >>>>> work as intented? >>>> >>>> Thanks for the heads up, no, unfortunately they don't work :-( >>>> >>>> They badly clash. I did the forward port of the three patches but they >>>> fail at runtime in my tests. I ran out of time to figure out what is the >>>> problem, and I'll have to pick it up at some point in the future (it >>>> might not be any time soon unfortunately). >>>> >>>> Rahul, if you have any ideas about what the problem is please let me >>>> know. This is the branch with the forward port: >>>> >>>> http://xenbits.xenproject.org/git-http/people/sstabellini/xen-unstable.git smmu-generic >>> >>> I did some more investigation and spotted a minor error in my forward >>> port. This an updated branch based on staging: >>> >>> http://xenbits.xenproject.org/git-http/people/sstabellini/xen-unstable.git smmu-generic-2 >>> >>> However, the real issue is that Rahul's patches break SMMU support on >>> ZynqMP even without my changes. I ran a bisection and found out that >>> patch #2 is the culprit: >>> >>> 5ee3fa0b21ea xen/arm: smmuv1: Consolidate stream map entry state >>> >>> It causes the abort appended below. The problem doesn't seem >>> particularly ZynqMP specific. Rahul, can you reproduce it on your side? >> >> Yes. I can reproduce the issue on Xilinx QEMU as we don’t have access to physical ZynqMP and found out that >> associating an iommu group pointer with the S2CR causing the issue. >> >> Associating the group pointer with S2CR is part of the patch "xen/arm: smmuv1: Intelligent SMR allocation”. >> >> I just revert that part of the code from the patch and it works fine for me. Please find the attached patch for the same. >> >> As per your analysis "5ee3fa0b21ea xen/arm: smmuv1: Consolidate stream map entry state” is causing the issue but what I found out that >> "xen/arm: smmuv1: Intelligent SMR allocation” is causing the issue. >> Can you please test it on the physical device and let me know if it works for you also to make sure we both observing the same issue. > > Great! Yes, I can confirm that your patch fixed the issue, now I can > boot staging on ZynqMP without errors and I can do device assignment > too. Thank you so much! > > The other good news is that the three "Generic SMMU Bindings" patches > work too on top of yours with the fix! > > Is the patch you submitted the valid fix for the problem? In other words, > should we go ahead, review, and commit the patch you attached or do you > want to send a different version of the patch for inclusion in Xen > staging? Thank you for testing the patch. Patch as it is ok for review. I will send the patch for review. Regards, Rahul
© 2016 - 2024 Red Hat, Inc.