[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2] xen/arm: fix gnttab_need_iommu_mapping
Hi,
On 10/02/2021 15:06, Rahul Singh wrote:
On 9 Feb 2021, at 8:36 pm, Stefano Stabellini <sstabellini@xxxxxxxxxx> wrote:
On Tue, 9 Feb 2021, Rahul Singh wrote:
On 8 Feb 2021, at 6:49 pm, Stefano Stabellini <sstabellini@xxxxxxxxxx> wrote:
Commit 91d4eca7add broke gnttab_need_iommu_mapping on ARM.
The offending chunk is:
#define gnttab_need_iommu_mapping(d) \
- (is_domain_direct_mapped(d) && need_iommu(d))
+ (is_domain_direct_mapped(d) && need_iommu_pt_sync(d))
On ARM we need gnttab_need_iommu_mapping to be true for dom0 when it is
directly mapped and IOMMU is enabled for the domain, like the old check
did, but the new check is always false.
In fact, need_iommu_pt_sync is defined as dom_iommu(d)->need_sync and
need_sync is set as:
if ( !is_hardware_domain(d) || iommu_hwdom_strict )
hd->need_sync = !iommu_use_hap_pt(d);
iommu_use_hap_pt(d) means that the page-table used by the IOMMU is the
P2M. It is true on ARM. need_sync means that you have a separate IOMMU
page-table and it needs to be updated for every change. need_sync is set
to false on ARM. Hence, gnttab_need_iommu_mapping(d) is false too,
which is wrong.
As a consequence, when using PV network from a domU on a system where
IOMMU is on from Dom0, I get:
(XEN) smmu: /smmu@fd800000: Unhandled context fault: fsr=0x402,
iova=0x8424cb148, fsynr=0xb0001, cb=0
[ 68.290307] macb ff0e0000.ethernet eth0: DMA bus error: HRESP not OK
The fix is to go back to something along the lines of the old
implementation of gnttab_need_iommu_mapping.
Signed-off-by: Stefano Stabellini <stefano.stabellini@xxxxxxxxxx>
Fixes: 91d4eca7add
Backport: 4.12+
---
Given the severity of the bug, I would like to request this patch to be
backported to 4.12 too, even if 4.12 is security-fixes only since Oct
2020.
For the 4.12 backport, we can use iommu_enabled() instead of
is_iommu_enabled() in the implementation of gnttab_need_iommu_mapping.
Changes in v2:
- improve commit message
- add is_iommu_enabled(d) to the check
---
xen/include/asm-arm/grant_table.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/xen/include/asm-arm/grant_table.h
b/xen/include/asm-arm/grant_table.h
index 6f585b1538..0ce77f9a1c 100644
--- a/xen/include/asm-arm/grant_table.h
+++ b/xen/include/asm-arm/grant_table.h
@@ -89,7 +89,7 @@ int replace_grant_host_mapping(unsigned long gpaddr, mfn_t
mfn,
(((i) >= nr_status_frames(t)) ? INVALID_GFN : (t)->arch.status_gfn[i])
#define gnttab_need_iommu_mapping(d) \
- (is_domain_direct_mapped(d) && need_iommu_pt_sync(d))
+ (is_domain_direct_mapped(d) && is_iommu_enabled(d))
#endif /* __ASM_GRANT_TABLE_H__ */
I tested the patch and while creating the guest I observed the below warning
from Linux for block device.
https://elixir.bootlin.com/linux/v4.3/source/drivers/block/xen-blkback/xenbus.c#L258
So you are creating a guest with "xl create" in dom0 and you see the
warnings below printed by the Dom0 kernel? I imagine the domU has a
virtual "disk" of some sort.
Yes you are right I am trying to create the guest with "xl create” and before
that, I created the logical volume and trying to attach the logical volume
block to the domain with “xl block-attach”. I observed this error with the "xl
block-attach” command.
This issue occurs after applying this patch as what I observed this patch
introduce the calls to iommu_legacy_{, un}map() to map the grant pages for
IOMMU that touches the page-tables. I am not sure but what I observed is that
something is written wrong when iomm_unmap calls unmap the pages
because of that issue is observed.
Can you clarify what you mean by "written wrong"? What sort of error do
you see in the iommu_unmap()?
You can reproduce the issue by just creating the dummy image file and try to attach it with
“xl block-attach”
dd if=/dev/zero of=test.img bs=1024 count=0 seek=1024
xl block-attach 0 phy:test.img xvda w
Sequence of command that I follow is as follows to reproduce the issue:
lvs vg-xen/myguest
lvcreate -y -L 4G -n myguest vg-xen
parted -s /dev/vg-xen/myguest mklabel msdos
parted -s /dev/vg-xen/myguest unit MB mkpart primary 1 4096
sync
xl block-attach 0 phy:/dev/vg-xen/myguest xvda w
libxl: error: libxl_xshelp.c:201:libxl__xs_read_mandatory: xenstore read
failed: `/libxl/0/type': No such file or directory
libxl: warning: libxl_dom.c:51:libxl__domain_type: unable to get domain type
for domid=0, assuming HVM
[ 162.632232] xen-blkback: backend/vbd/0/51712: using 4 queues, protocol 1
(arm-abi) persistent grants
[ 162.764847] vbd vbd-0-51712: 9 mapping in shared page 8 from domain 0
[ 162.771740] vbd vbd-0-51712: 9 mapping ring-ref port 5
[ 162.777650] ------------[ cut here ]------------
[ 162.782167] WARNING: CPU: 2 PID: 37 at
drivers/block/xen-blkback/xenbus.c:296 xen_blkif_disconnect+0x20c/0x230
Just to confirm, this splat comes from:
WARN_ON(i != (XEN_BLKIF_REQS_PER_PAGE * blkif->nr_ring_pages));
If so, what are the values for i and blkif->nr_ring_pages?
Cheers,
--
Julien Grall
|
