From: Rahul Singh <Rahul.Singh@arm.com>
To: Stefano Stabellini <sstabellini@kernel.org>
Cc: Julien Grall <julien@xen.org>,
xen-devel <xen-devel@lists.xenproject.org>,
Bertrand Marquis <Bertrand.Marquis@arm.com>,
Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>,
"brian.woods@xilinx.com" <brian.woods@xilinx.com>
Subject: Re: [PATCH v3 0/3] Generic SMMU Bindings
Date: Mon, 12 Apr 2021 10:20:16 +0000 [thread overview]
Message-ID: <A64881AF-DAD6-4A14-A545-354A75FE1B4F@arm.com> (raw)
In-Reply-To: <alpine.DEB.2.21.2104091714270.4885@sstabellini-ThinkPad-T480s>
[-- Attachment #1.1: Type: text/plain, Size: 8324 bytes --]
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
[-- Attachment #1.2: Type: text/html, Size: 11373 bytes --]
[-- Attachment #2: 0001-xen-arm-smmuv1-Revert-associating-the-group-pointer-.patch --]
[-- Type: application/octet-stream, Size: 2959 bytes --]
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
next prev parent reply other threads:[~2021-04-12 10:21 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-01-26 22:58 [PATCH v3 0/3] Generic SMMU Bindings Stefano Stabellini
2021-01-26 22:58 ` [PATCH v3 1/3] arm,smmu: switch to using iommu_fwspec functions Stefano Stabellini
2021-02-02 16:07 ` Rahul Singh
2021-01-26 22:58 ` [PATCH v3 2/3] arm,smmu: restructure code in preparation to new bindings support Stefano Stabellini
2021-02-02 16:09 ` Rahul Singh
2021-01-26 22:58 ` [PATCH v3 3/3] arm,smmu: add support for generic DT bindings. Implement add_device and dt_xlate Stefano Stabellini
2021-02-02 16:09 ` Rahul Singh
2021-02-02 16:07 ` [PATCH v3 0/3] Generic SMMU Bindings Rahul Singh
2021-02-02 17:44 ` Stefano Stabellini
2021-02-02 17:50 ` Julien Grall
2021-02-02 18:27 ` Stefano Stabellini
2021-02-02 18:43 ` Julien Grall
2021-02-03 17:14 ` Rahul Singh
2021-04-05 15:23 ` Julien Grall
2021-04-06 23:59 ` Stefano Stabellini
2021-04-07 15:43 ` Rahul Singh
2021-04-10 0:27 ` Stefano Stabellini
2021-04-12 10:20 ` Rahul Singh [this message]
2021-04-13 0:27 ` Stefano Stabellini
2021-04-14 10:42 ` Rahul Singh
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=A64881AF-DAD6-4A14-A545-354A75FE1B4F@arm.com \
--to=rahul.singh@arm.com \
--cc=Bertrand.Marquis@arm.com \
--cc=Volodymyr_Babchuk@epam.com \
--cc=brian.woods@xilinx.com \
--cc=julien@xen.org \
--cc=sstabellini@kernel.org \
--cc=xen-devel@lists.xenproject.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).