RE: [PATCH v2 14/15] vfio: Document dual stage control
> From: Stefan Hajnoczi
> Sent: Monday, June 15, 2020 5:41 PM
> On Thu, Jun 11, 2020 at 05:15:33AM -0700, Liu Yi L wrote:
>
> > From: Eric Auger
> >
> > The VFIO API was enhanced to support nested stage control: a bunch of
> > new iotcls and usage guideline.
> >
> > Let's document the process to follow to set up nested mode.
> >
> > Cc: Kevin Tian
> > CC: Jacob Pan
> > Cc: Alex Williamson
> > Cc: Eric Auger
> > Cc: Jean-Philippe Brucker
> > Cc: Joerg Roedel
> > Cc: Lu Baolu
> > Signed-off-by: Eric Auger
> > Signed-off-by: Liu Yi L
> > ---
> > v1 -> v2:
> > *) new in v2, compared with Eric's original version, pasid table bind
> >and fault reporting is removed as this series doesn't cover them.
> >Original version from Eric.
> >https://lkml.org/lkml/2020/3/20/700
> >
> > Documentation/driver-api/vfio.rst | 64
> > +++
> > 1 file changed, 64 insertions(+)
> >
> > diff --git a/Documentation/driver-api/vfio.rst
> > b/Documentation/driver-api/vfio.rst
> > index f1a4d3c..06224bd 100644
> > --- a/Documentation/driver-api/vfio.rst
> > +++ b/Documentation/driver-api/vfio.rst
> > @@ -239,6 +239,70 @@ group and can access them as follows::
> > /* Gratuitous device reset and go... */
> > ioctl(device, VFIO_DEVICE_RESET);
> >
> > +IOMMU Dual Stage Control
> > +
> > +
> > +Some IOMMUs support 2 stages/levels of translation. Stage corresponds
> > +to the ARM terminology while level corresponds to Intel's VTD terminology.
> > +In the following text we use either without distinction.
> > +
> > +This is useful when the guest is exposed with a virtual IOMMU and
> > +some devices are assigned to the guest through VFIO. Then the guest
> > +OS can use stage 1 (GIOVA -> GPA or GVA->GPA), while the hypervisor
> > +uses stage 2 for VM isolation (GPA -> HPA).
> > +
> > +Under dual stage translation, the guest gets ownership of the stage 1
> > +page tables and also owns stage 1 configuration structures. The
> > +hypervisor owns the root configuration structure (for security
> > +reason), including stage 2 configuration. This works as long
> > +configuration structures and page table
>
> s/as long configuration/as long as configuration/
got it.
>
> > +format are compatible between the virtual IOMMU and the physical IOMMU.
>
> s/format/formats/
I see.
> > +
> > +Assuming the HW supports it, this nested mode is selected by choosing
> > +the VFIO_TYPE1_NESTING_IOMMU type through:
> > +
> > +ioctl(container, VFIO_SET_IOMMU, VFIO_TYPE1_NESTING_IOMMU);
> > +
> > +This forces the hypervisor to use the stage 2, leaving stage 1
> > +available for guest usage. The guest stage 1 format depends on IOMMU
> > +vendor, and it is the same with the nesting configuration method.
> > +User space should check the format and configuration method after
> > +setting nesting type by
> > +using:
> > +
> > +ioctl(container->fd, VFIO_IOMMU_GET_INFO, &nesting_info);
> > +
> > +Details can be found in Documentation/userspace-api/iommu.rst. For
> > +Intel VT-d, each stage 1 page table is bound to host by:
> > +
> > +nesting_op->flags = VFIO_IOMMU_NESTING_OP_BIND_PGTBL;
> > +memcpy(&nesting_op->data, &bind_data, sizeof(bind_data));
> > +ioctl(container->fd, VFIO_IOMMU_NESTING_OP, nesting_op);
> > +
> > +As mentioned above, guest OS may use stage 1 for GIOVA->GPA or GVA->GPA.
> > +GVA->GPA page tables are available when PASID (Process Address Space
> > +GVA->ID)
> > +is exposed to guest. e.g. guest with PASID-capable devices assigned.
> > +For such page table binding, the bind_data should include PASID info,
> > +which is allocated by guest itself or by host. This depends on
> > +hardware vendor e.g. Intel VT-d requires to allocate PASID from host.
> > +This requirement is available by VFIO_IOMMU_GET_INFO. User space
> > +could allocate PASID from host by:
> > +
> > +req.flags = VFIO_IOMMU_ALLOC_PASID;
> > +ioctl(container, VFIO_IOMMU_PASID_REQUEST, &req);
>
> It is not clear how the userspace application determines whether PASIDs must
> be
> allocated from the host via VFIO_IOMMU_PASID_REQUEST or if the guest itself
> can
> allocate PASIDs. The text mentions VFIO_IOMMU_GET_INFO but what exactly
> should the userspace application check?
For VT-d, spec 3.0 introduced Virtual Cmd interface for PASID allocation,
guest request PASID from host if it detects the interface. Application
should check the IOMMU_NESTING_FEAT_SYSWIDE_PASID setting in the below
info reported by VFIO_IOMMU_GET_INFO. And virtual VT-d should not report
SVA related capabilities to guest if SYSWIDE_PASID is not supported by
kernel.
+struct iommu_nesting_info {
+ __u32 size;
+ __u32 format;
+ __u32 features;
+#define IOMMU_NESTING_FEAT_SYSWIDE_PASID (1 << 0)
+#define IOMMU_NESTING_FEAT_BIND_PGTBL (1 << 1)
+#define IOMMU_NESTING_FEAT_CACHE_INVLD (1 << 2)
+ __u32 flags;
+ __u8data[];
+};
https://lore.kernel.o
RE: [PATCH v2 1/3] docs: IOMMU user API
> From: Jacob Pan > Sent: Tuesday, June 16, 2020 11:22 PM > > On Thu, 11 Jun 2020 17:27:27 -0700 > Jacob Pan wrote: > > > > > > > But then I thought it even better if VFIO leaves the entire > > > copy_from_user() to the layer consuming it. > > > > > OK. Sounds good, that was what Kevin suggested also. I just wasn't > > sure how much VFIO wants to inspect, I thought VFIO layer wanted to do > > a sanity check. > > > > Anyway, I will move copy_from_user to iommu uapi layer. > > Just one more point brought up by Yi when we discuss this offline. > > If we move copy_from_user to iommu uapi layer, then there will be multiple > copy_from_user calls for the same data when a VFIO container has multiple > domains, > devices. For bind, it might be OK. But might be additional overhead for TLB > flush > request from the guest. I think it is the same with bind and TLB flush path. will be multiple copy_from_user. BTW. for moving data copy to iommy layer, there is another point which need to consider. VFIO needs to do unbind in bind path if bind failed, so it will assemble unbind_data and pass to iommu layer. If iommu layer do the copy_from_user, I think it will be failed. any idea? Regards, Yi Liu > Thoughts? > > Jacob ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v4 01/17] media: dt-binding: mtk-vcodec: Separating mtk-vcodec encode node.
On Wed, 2020-06-10 at 15:38 +0800, Tiffany Lin wrote:
> On Wed, 2020-06-10 at 15:46 +0900, Alexandre Courbot wrote:
> > On Wed, Jun 10, 2020 at 6:21 AM Rob Herring wrote:
> > >
> > > On Sat, May 30, 2020 at 04:10:02PM +0800, Yong Wu wrote:
> > > > From: Maoguang Meng
> > > >
> > > > Update binding document since the avc and vp8 hardware encoder in
> > > > mt8173 are now separated. Separate "mediatek,mt8173-vcodec-enc" to
> > > > "mediatek,mt8173-vcodec-vp8-enc" and "mediatek,mt8173-vcodec-avc-enc".
> > >
> > > The h/w suddenly split in 2? You are breaking compatibility. Up to the
> > > Mediatek maintainers to decide if that's okay, but you need to state you
> > > are breaking compatibility (here and in the driver) and why that is
> > > okay.
> >
> > In my understanding there is no real hardware using the old bindings
> > at the moment, and the split is indeed a reflection of the actual
> > hardware layout. Tiffany, can you give your acked-by if this change is
> > ok with you?
> >
>
> In my opinion, there is no need to change mt8173 dts for driver to
> support mt8183.
> I saw another patch that already make change to have encoder driver
> support both mt8173 and mt8183.
> But they done a lot to prove h264 and vp8 encoder could work
> independently and parallel.
> In this case, I am ok with it because dts should be a reflection of the
> actual hardware.
>
>
>
> > >
> > > >
> > > > This is a preparing patch for smi cleaning up "mediatek,larb".
> > > >
Acked-by: Tiffany Lin
> > > > Signed-off-by: Maoguang Meng
> > > > Signed-off-by: Hsin-Yi Wang
> > > > Signed-off-by: Irui Wang
> > > > Signed-off-by: Yong Wu
> > > > ---
> > > > .../devicetree/bindings/media/mediatek-vcodec.txt | 58
> > > > --
> > > > 1 file changed, 31 insertions(+), 27 deletions(-)
> > > >
> > > > diff --git
> > > > a/Documentation/devicetree/bindings/media/mediatek-vcodec.txt
> > > > b/Documentation/devicetree/bindings/media/mediatek-vcodec.txt
> > > > index 8093335..1023740 100644
> > > > --- a/Documentation/devicetree/bindings/media/mediatek-vcodec.txt
> > > > +++ b/Documentation/devicetree/bindings/media/mediatek-vcodec.txt
> > > > @@ -4,7 +4,9 @@ Mediatek Video Codec is the video codec hw present in
> > > > Mediatek SoCs which
> > > > supports high resolution encoding and decoding functionalities.
> > > >
> > > > Required properties:
> > > > -- compatible : "mediatek,mt8173-vcodec-enc" for MT8173 encoder
> > > > +- compatible : must be one of the following string:
> > > > + "mediatek,mt8173-vcodec-vp8-enc" for mt8173 vp8 encoder.
> > > > + "mediatek,mt8173-vcodec-avc-enc" for mt8173 avc encoder.
> > > >"mediatek,mt8183-vcodec-enc" for MT8183 encoder.
> > > >"mediatek,mt8173-vcodec-dec" for MT8173 decoder.
> > > > - reg : Physical base address of the video codec registers and length
> > > > of
> > > > @@ -13,10 +15,11 @@ Required properties:
> > > > - mediatek,larb : must contain the local arbiters in the current Socs.
> > > > - clocks : list of clock specifiers, corresponding to entries in
> > > >the clock-names property.
> > > > -- clock-names: encoder must contain "venc_sel_src", "venc_sel",,
> > > > - "venc_lt_sel_src", "venc_lt_sel", decoder must contain "vcodecpll",
> > > > - "univpll_d2", "clk_cci400_sel", "vdec_sel", "vdecpll", "vencpll",
> > > > - "venc_lt_sel", "vdec_bus_clk_src".
> > > > +- clock-names:
> > > > + avc venc must contain "venc_sel";
> > > > + vp8 venc must contain "venc_lt_sel";
> > > > + decoder must contain "vcodecpll", "univpll_d2", "clk_cci400_sel",
> > > > + "vdec_sel", "vdecpll", "vencpll", "venc_lt_sel", "vdec_bus_clk_src".
> > > > - iommus : should point to the respective IOMMU block with master port
> > > > as
> > > >argument, see
> > > > Documentation/devicetree/bindings/iommu/mediatek,iommu.txt
> > > >for details.
> > > > @@ -80,14 +83,10 @@ vcodec_dec: vcodec@1600 {
> > > > assigned-clock-rates = <0>, <0>, <0>, <148200>, <8>;
> > > >};
> > > >
> > > > - vcodec_enc: vcodec@18002000 {
> > > > -compatible = "mediatek,mt8173-vcodec-enc";
> > > > -reg = <0 0x18002000 0 0x1000>,/*VENC_SYS*/
> > > > - <0 0x19002000 0 0x1000>;/*VENC_LT_SYS*/
> > > > -interrupts = ,
> > > > - ;
> > > > -mediatek,larb = <&larb3>,
> > > > - <&larb5>;
> > > > +vcodec_enc: vcodec@18002000 {
> > > > +compatible = "mediatek,mt8173-vcodec-avc-enc";
> > > > +reg = <0 0x18002000 0 0x1000>;
> > > > +interrupts = ;
> > > > iommus = <&iommu M4U_PORT_VENC_RCPU>,
> > > > <&iommu M4U_PORT_VENC_REC>,
> > > > <&iommu M4U_PORT_VENC_BSDMA>,
> > > > @@ -98,8 +97,20 @@ vcodec_dec: vcodec@1600 {
> > > > <&iommu M4U_PORT_VENC_REF_LUMA>,
> > > > <&iommu M4U_PORT_VENC_REF_CHROMA>,
> > > > <&iommu M4U_PORT_VENC_NBM_RDMA>,
> > > > - <&iommu M4U_PORT_VENC_NBM_WDMA>,
> > > > -
[PATCH v4 7/7] iommu/mediatek: Add mt6779 basic support
1. Start from mt6779, INVLDT_SEL move to offset=0x2c, so we add
REG_MMU_INV_SEL_GEN2 definition and mt6779 uses it.
2. Change PROTECT_PA_ALIGN from 128 byte to 256 byte.
3. For REG_MMU_CTRL_REG register, we only need to change bit[2:0],
others bits keep default value, ex: enable victim tlb.
4. Add mt6779_data to support mm_iommu HW init.
Change since v3:
1. When setting MMU_CTRL_REG, we don't need to include mt8173.
Cc: Yong Wu
Signed-off-by: Chao Hao
---
drivers/iommu/mtk_iommu.c | 20 ++--
drivers/iommu/mtk_iommu.h | 1 +
2 files changed, 19 insertions(+), 2 deletions(-)
diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
index c706bca6487e..def2e996683f 100644
--- a/drivers/iommu/mtk_iommu.c
+++ b/drivers/iommu/mtk_iommu.c
@@ -37,6 +37,11 @@
#define REG_MMU_INVLD_START_A 0x024
#define REG_MMU_INVLD_END_A0x028
+/* In latest Coda, MMU_INV_SEL's offset is changed to 0x02c.
+ * So we named offset = 0x02c to "REG_MMU_INV_SEL_GEN2"
+ * and offset = 0x038 to "REG_MMU_INV_SEL_GEN1".
+ */
+#define REG_MMU_INV_SEL_GEN2 0x02c
#define REG_MMU_INV_SEL_GEN1 0x038
#define F_INVLD_EN0BIT(0)
#define F_INVLD_EN1BIT(1)
@@ -98,7 +103,7 @@
#define F_MMU_INT_ID_LARB_ID(a)(((a) >> 7) & 0x7)
#define F_MMU_INT_ID_PORT_ID(a)(((a) >> 2) & 0x1f)
-#define MTK_PROTECT_PA_ALIGN 128
+#define MTK_PROTECT_PA_ALIGN 256
/*
* Get the local arbiter ID and the portid within the larb arbiter
@@ -543,11 +548,12 @@ static int mtk_iommu_hw_init(const struct mtk_iommu_data
*data)
return ret;
}
+ regval = readl_relaxed(data->base + REG_MMU_CTRL_REG);
if (data->plat_data->m4u_plat == M4U_MT8173)
regval = F_MMU_PREFETCH_RT_REPLACE_MOD |
F_MMU_TF_PROT_TO_PROGRAM_ADDR_MT8173;
else
- regval = F_MMU_TF_PROT_TO_PROGRAM_ADDR;
+ regval |= F_MMU_TF_PROT_TO_PROGRAM_ADDR;
writel_relaxed(regval, data->base + REG_MMU_CTRL_REG);
regval = F_L2_MULIT_HIT_EN |
@@ -797,6 +803,15 @@ static const struct mtk_iommu_plat_data mt2712_data = {
.larbid_remap = {{0}, {1}, {2}, {3}, {4}, {5}, {6}, {7}},
};
+static const struct mtk_iommu_plat_data mt6779_data = {
+ .m4u_plat = M4U_MT6779,
+ .has_sub_comm = true,
+ .has_wr_len= true,
+ .has_misc_ctrl = true,
+ .inv_sel_reg = REG_MMU_INV_SEL_GEN2,
+ .larbid_remap = {{0}, {1}, {2}, {3}, {5}, {7, 8}, {10}, {9}},
+};
+
static const struct mtk_iommu_plat_data mt8173_data = {
.m4u_plat = M4U_MT8173,
.has_4gb_mode = true,
@@ -815,6 +830,7 @@ static const struct mtk_iommu_plat_data mt8183_data = {
static const struct of_device_id mtk_iommu_of_ids[] = {
{ .compatible = "mediatek,mt2712-m4u", .data = &mt2712_data},
+ { .compatible = "mediatek,mt6779-m4u", .data = &mt6779_data},
{ .compatible = "mediatek,mt8173-m4u", .data = &mt8173_data},
{ .compatible = "mediatek,mt8183-m4u", .data = &mt8183_data},
{}
diff --git a/drivers/iommu/mtk_iommu.h b/drivers/iommu/mtk_iommu.h
index 9971cedd72ea..fb79e710c8d9 100644
--- a/drivers/iommu/mtk_iommu.h
+++ b/drivers/iommu/mtk_iommu.h
@@ -31,6 +31,7 @@ struct mtk_iommu_suspend_reg {
enum mtk_iommu_plat {
M4U_MT2701,
M4U_MT2712,
+ M4U_MT6779,
M4U_MT8173,
M4U_MT8183,
};
--
2.18.0
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v4 6/7] iommu/mediatek: Add REG_MMU_WR_LEN definition preparing for mt6779
Some platforms(ex: mt6779) have a new register called by REG_MMU_WR_LEN
to improve performance.
This patch add this register definition.
Signed-off-by: Chao Hao
---
drivers/iommu/mtk_iommu.c | 10 ++
drivers/iommu/mtk_iommu.h | 2 ++
2 files changed, 12 insertions(+)
diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
index a687e8db0e51..c706bca6487e 100644
--- a/drivers/iommu/mtk_iommu.c
+++ b/drivers/iommu/mtk_iommu.c
@@ -46,6 +46,8 @@
#define F_MMU_STANDARD_AXI_MODE_BIT(BIT(3) | BIT(19))
#define REG_MMU_DCM_DIS0x050
+#define REG_MMU_WR_LEN 0x054
+#define F_MMU_WR_THROT_DIS_BIT (BIT(5) | BIT(21))
#define REG_MMU_CTRL_REG 0x110
#define F_MMU_TF_PROT_TO_PROGRAM_ADDR (2 << 4)
@@ -581,6 +583,12 @@ static int mtk_iommu_hw_init(const struct mtk_iommu_data
*data)
writel_relaxed(regval, data->base + REG_MMU_VLD_PA_RNG);
}
writel_relaxed(0, data->base + REG_MMU_DCM_DIS);
+ if (data->plat_data->has_wr_len) {
+ /* write command throttling mode */
+ regval = readl_relaxed(data->base + REG_MMU_WR_LEN);
+ regval &= ~F_MMU_WR_THROT_DIS_BIT;
+ writel_relaxed(regval, data->base + REG_MMU_WR_LEN);
+ }
if (data->plat_data->reset_axi) {
/* The register is called STANDARD_AXI_MODE in this case */
@@ -737,6 +745,7 @@ static int __maybe_unused mtk_iommu_suspend(struct device
*dev)
struct mtk_iommu_suspend_reg *reg = &data->reg;
void __iomem *base = data->base;
+ reg->wr_len = readl_relaxed(base + REG_MMU_WR_LEN);
reg->misc_ctrl = readl_relaxed(base + REG_MMU_MISC_CTRL);
reg->dcm_dis = readl_relaxed(base + REG_MMU_DCM_DIS);
reg->ctrl_reg = readl_relaxed(base + REG_MMU_CTRL_REG);
@@ -761,6 +770,7 @@ static int __maybe_unused mtk_iommu_resume(struct device
*dev)
dev_err(data->dev, "Failed to enable clk(%d) in resume\n", ret);
return ret;
}
+ writel_relaxed(reg->wr_len, base + REG_MMU_WR_LEN);
writel_relaxed(reg->misc_ctrl, base + REG_MMU_MISC_CTRL);
writel_relaxed(reg->dcm_dis, base + REG_MMU_DCM_DIS);
writel_relaxed(reg->ctrl_reg, base + REG_MMU_CTRL_REG);
diff --git a/drivers/iommu/mtk_iommu.h b/drivers/iommu/mtk_iommu.h
index d51ff99c2c71..9971cedd72ea 100644
--- a/drivers/iommu/mtk_iommu.h
+++ b/drivers/iommu/mtk_iommu.h
@@ -25,6 +25,7 @@ struct mtk_iommu_suspend_reg {
u32 int_main_control;
u32 ivrp_paddr;
u32 vld_pa_rng;
+ u32 wr_len;
};
enum mtk_iommu_plat {
@@ -43,6 +44,7 @@ struct mtk_iommu_plat_data {
boolhas_misc_ctrl;
boolhas_sub_comm;
boolhas_vld_pa_rng;
+ boolhas_wr_len;
boolreset_axi;
u32 inv_sel_reg;
unsigned char larbid_remap[8][4];
--
2.18.0
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v4 5/7] iommu/mediatek: Add sub_comm id in translation fault
The max larb number that a iommu HW support is 8(larb0~larb7 in the below
diagram).
If the larb's number is over 8, we use a sub_common for merging
several larbs into one larb. At this case, we will extend larb_id:
bit[11:9] means common-id;
bit[8:7] means subcommon-id;
>From these two variable, we could get the real larb number when
translation fault happen.
The diagram is as below:
EMI
|
IOMMU
|
-
| |
common1 common0
| |
-
|
smi common
|
| | | | ||
3'd03'd13'd23'd3 ... 3'd7 <-common_id(max is 8)
| | | | ||
Larb0 Larb1 | Larb3 ... Larb7
|
smi sub common
|
--
|| | |
2'd0 2'd12'd22'd3 <-sub_common_id(max is 4)
|| | |
Larb8Larb9 Larb10 Larb11
In this patch we extern larb_remap[] to larb_remap[8][4] for this.
larb_remap[x][y]: x mean common-id above, y means subcommon_id above.
We can also distinguish if the M4U HW has sub_common by has_sub_comm
property.
Signed-off-by: Chao Hao
Reviewed-by: Yong Wu
---
drivers/iommu/mtk_iommu.c | 20 +---
drivers/iommu/mtk_iommu.h | 3 ++-
2 files changed, 15 insertions(+), 8 deletions(-)
diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
index f23919feba4e..a687e8db0e51 100644
--- a/drivers/iommu/mtk_iommu.c
+++ b/drivers/iommu/mtk_iommu.c
@@ -91,6 +91,8 @@
#define REG_MMU1_INVLD_PA 0x148
#define REG_MMU0_INT_ID0x150
#define REG_MMU1_INT_ID0x154
+#define F_MMU_INT_ID_COMM_ID(a)(((a) >> 9) & 0x7)
+#define F_MMU_INT_ID_SUB_COMM_ID(a)(((a) >> 7) & 0x3)
#define F_MMU_INT_ID_LARB_ID(a)(((a) >> 7) & 0x7)
#define F_MMU_INT_ID_PORT_ID(a)(((a) >> 2) & 0x1f)
@@ -229,7 +231,7 @@ static irqreturn_t mtk_iommu_isr(int irq, void *dev_id)
struct mtk_iommu_data *data = dev_id;
struct mtk_iommu_domain *dom = data->m4u_dom;
u32 int_state, regval, fault_iova, fault_pa;
- unsigned int fault_larb, fault_port;
+ unsigned int fault_larb, fault_port, sub_comm = 0;
bool layer, write;
/* Read error info from registers */
@@ -245,10 +247,14 @@ static irqreturn_t mtk_iommu_isr(int irq, void *dev_id)
}
layer = fault_iova & F_MMU_FAULT_VA_LAYER_BIT;
write = fault_iova & F_MMU_FAULT_VA_WRITE_BIT;
- fault_larb = F_MMU_INT_ID_LARB_ID(regval);
fault_port = F_MMU_INT_ID_PORT_ID(regval);
-
- fault_larb = data->plat_data->larbid_remap[fault_larb];
+ if (data->plat_data->has_sub_comm) {
+ fault_larb = F_MMU_INT_ID_COMM_ID(regval);
+ sub_comm = F_MMU_INT_ID_SUB_COMM_ID(regval);
+ } else {
+ fault_larb = F_MMU_INT_ID_LARB_ID(regval);
+ }
+ fault_larb = data->plat_data->larbid_remap[fault_larb][sub_comm];
if (report_iommu_fault(&dom->domain, data->dev, fault_iova,
write ? IOMMU_FAULT_WRITE : IOMMU_FAULT_READ)) {
@@ -778,7 +784,7 @@ static const struct mtk_iommu_plat_data mt2712_data = {
.has_bclk = true,
.has_vld_pa_rng = true,
.inv_sel_reg= REG_MMU_INV_SEL_GEN1,
- .larbid_remap = {0, 1, 2, 3, 4, 5, 6, 7, 8, 9},
+ .larbid_remap = {{0}, {1}, {2}, {3}, {4}, {5}, {6}, {7}},
};
static const struct mtk_iommu_plat_data mt8173_data = {
@@ -787,14 +793,14 @@ static const struct mtk_iommu_plat_data mt8173_data = {
.has_bclk = true,
.reset_axi= true,
.inv_sel_reg = REG_MMU_INV_SEL_GEN1,
- .larbid_remap = {0, 1, 2, 3, 4, 5}, /* Linear mapping. */
+ .larbid_remap = {{0}, {1}, {2}, {3}, {4}, {5}}, /* Linear mapping. */
};
static const struct mtk_iommu_plat_data mt8183_data = {
.m4u_plat = M4U_MT8183,
.reset_axi= true,
.inv_sel_reg = REG_MMU_INV_SEL_GEN1,
- .larbid_remap = {0, 4, 5, 6, 7, 2, 3, 1},
+ .larbid_remap = {{0}, {4}, {5}, {6}, {7}, {2}, {3}, {1}},
};
static const struct of_device_id mtk_iommu_of_ids[] = {
diff --git a/drivers/iommu/mtk_iommu.h b/drivers/iommu/mtk_iommu.h
index afd7a2de5c1e..d51ff99c2c71 100644
--- a/drivers/iommu/mtk_iommu.h
+++ b/drivers/iommu/mtk_iommu.h
@@ -41,10 +41,11 @@ struct mtk_iommu_plat_data {
/* HW will use the EMI clock if there isn't the "bclk". */
boolhas_bclk;
boolhas_misc_ctrl;
+ boolhas_sub_comm;
boolhas_vld_pa_rng;
[PATCH v4 1/7] dt-bindings: mediatek: Add bindings for MT6779
This patch adds description for MT6779 IOMMU. MT6779 has two iommus, they are mm_iommu and apu_iommu which both use ARM Short-Descriptor translation format. In addition, mm_iommu and apu_iommu are two independent HW instance , we need to set them separately. The MT6779 IOMMU hardware diagram is as below, it is only a brief diagram about iommu, it don't focus on the part of smi_larb, so I don't describe the smi_larb detailedly. EMI | -- || MM_IOMMUAPU_IOMMU || SMI_COMMOM--- APU_BUS ||| SMI_LARB(0~11) || ||| || -- || | | | Multimedia engine CCU VPU MDLA EMDA All the connections are hardware fixed, software can not adjust it. Change since v2: 1. Delete unused definition, ex: M4U_LARB12_ID, M4U_LARB13_ID, CCU, VPU, MDLA, EDMA Change since v1: 1. Delete M4U_PORT_UNKNOWN define because of not use it. 2. Correct coding format: ex: /*larb3-VENC*/ --> /* larb3-VENC */ Signed-off-by: Chao Hao Reviewed-by: Rob Herring --- .../bindings/iommu/mediatek,iommu.txt | 2 + include/dt-bindings/memory/mt6779-larb-port.h | 206 ++ 2 files changed, 208 insertions(+) create mode 100644 include/dt-bindings/memory/mt6779-larb-port.h diff --git a/Documentation/devicetree/bindings/iommu/mediatek,iommu.txt b/Documentation/devicetree/bindings/iommu/mediatek,iommu.txt index ce59a505f5a4..c1ccd8582eb2 100644 --- a/Documentation/devicetree/bindings/iommu/mediatek,iommu.txt +++ b/Documentation/devicetree/bindings/iommu/mediatek,iommu.txt @@ -58,6 +58,7 @@ Required properties: - compatible : must be one of the following string: "mediatek,mt2701-m4u" for mt2701 which uses generation one m4u HW. "mediatek,mt2712-m4u" for mt2712 which uses generation two m4u HW. + "mediatek,mt6779-m4u" for mt6779 which uses generation two m4u HW. "mediatek,mt7623-m4u", "mediatek,mt2701-m4u" for mt7623 which uses generation one m4u HW. "mediatek,mt8173-m4u" for mt8173 which uses generation two m4u HW. @@ -78,6 +79,7 @@ Required properties: Specifies the mtk_m4u_id as defined in dt-binding/memory/mt2701-larb-port.h for mt2701, mt7623 dt-binding/memory/mt2712-larb-port.h for mt2712, + dt-binding/memory/mt6779-larb-port.h for mt6779, dt-binding/memory/mt8173-larb-port.h for mt8173, and dt-binding/memory/mt8183-larb-port.h for mt8183. diff --git a/include/dt-bindings/memory/mt6779-larb-port.h b/include/dt-bindings/memory/mt6779-larb-port.h new file mode 100644 index ..2ad0899fbf2f --- /dev/null +++ b/include/dt-bindings/memory/mt6779-larb-port.h @@ -0,0 +1,206 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/* + * Copyright (c) 2019 MediaTek Inc. + * Author: Chao Hao + */ + +#ifndef _DTS_IOMMU_PORT_MT6779_H_ +#define _DTS_IOMMU_PORT_MT6779_H_ + +#define MTK_M4U_ID(larb, port) (((larb) << 5) | (port)) + +#define M4U_LARB0_ID0 +#define M4U_LARB1_ID1 +#define M4U_LARB2_ID2 +#define M4U_LARB3_ID3 +#define M4U_LARB4_ID4 +#define M4U_LARB5_ID5 +#define M4U_LARB6_ID6 +#define M4U_LARB7_ID7 +#define M4U_LARB8_ID8 +#define M4U_LARB9_ID9 +#define M4U_LARB10_ID 10 +#define M4U_LARB11_ID 11 + +/* larb0 */ +#define M4U_PORT_DISP_POSTMASK0 MTK_M4U_ID(M4U_LARB0_ID, 0) +#define M4U_PORT_DISP_OVL0_HDR MTK_M4U_ID(M4U_LARB0_ID, 1) +#define M4U_PORT_DISP_OVL1_HDR MTK_M4U_ID(M4U_LARB0_ID, 2) +#define M4U_PORT_DISP_OVL0 MTK_M4U_ID(M4U_LARB0_ID, 3) +#define M4U_PORT_DISP_OVL1 MTK_M4U_ID(M4U_LARB0_ID, 4) +#define M4U_PORT_DISP_PVRIC0MTK_M4U_ID(M4U_LARB0_ID, 5) +#define M4U_PORT_DISP_RDMA0 MTK_M4U_ID(M4U_LARB0_ID, 6) +#define M4U_PORT_DISP_WDMA0 MTK_M4U_ID(M4U_LARB0_ID, 7) +#define M4U_PORT_DISP_FAKE0 MTK_M4U_ID(M4U_LARB0_ID, 8) + +/* larb1 */ +#define M4U_PORT_DISP_OVL0_2L_HDR MTK_M4U_ID(M4U_LARB1_ID, 0) +#define M4U_PORT_DISP_OVL1_2L_HDR MTK_M4U_ID(M4U_LARB1_ID, 1) +#define M4U_PORT_DISP_OVL0_2L MTK_M4U_ID(M4U_LARB1_ID, 2) +#define M4U_PORT_DISP_OVL1_2L MTK_M4U_ID(M4U_LARB1_ID, 3) +#define M4U_PORT_DISP_RDMA1 MTK_M4U_ID(M4U_LARB1_ID, 4) +#define M4U_PORT_MDP_PVRIC0 MTK_M4U_ID(M4U_LARB1_ID, 5)
[PATCH v4 4/7] iommu/mediatek: Move inv_sel_reg into the plat_data
For mt6779, MMU_INV_SEL register's offset is changed from
0x38 to 0x2c, so we can put inv_sel_reg in the plat_data to
use it.
In addition, we renamed it to REG_MMU_INV_SEL_GEN1 and use it
before mt6779.
Change since v3:
1. Fix coding style
Cc: Yong Wu
Signed-off-by: Chao Hao
---
drivers/iommu/mtk_iommu.c | 19 +++
drivers/iommu/mtk_iommu.h | 1 +
2 files changed, 12 insertions(+), 8 deletions(-)
diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
index 239d2cdbbc9f..f23919feba4e 100644
--- a/drivers/iommu/mtk_iommu.c
+++ b/drivers/iommu/mtk_iommu.c
@@ -37,7 +37,7 @@
#define REG_MMU_INVLD_START_A 0x024
#define REG_MMU_INVLD_END_A0x028
-#define REG_MMU_INV_SEL0x038
+#define REG_MMU_INV_SEL_GEN1 0x038
#define F_INVLD_EN0BIT(0)
#define F_INVLD_EN1BIT(1)
@@ -168,7 +168,7 @@ static void mtk_iommu_tlb_flush_all(void *cookie)
for_each_m4u(data) {
writel_relaxed(F_INVLD_EN1 | F_INVLD_EN0,
- data->base + REG_MMU_INV_SEL);
+ data->base + data->plat_data->inv_sel_reg);
writel_relaxed(F_ALL_INVLD, data->base + REG_MMU_INVALIDATE);
wmb(); /* Make sure the tlb flush all done */
}
@@ -185,7 +185,7 @@ static void mtk_iommu_tlb_flush_range_sync(unsigned long
iova, size_t size,
for_each_m4u(data) {
spin_lock_irqsave(&data->tlb_lock, flags);
writel_relaxed(F_INVLD_EN1 | F_INVLD_EN0,
- data->base + REG_MMU_INV_SEL);
+ data->base + data->plat_data->inv_sel_reg);
writel_relaxed(iova, data->base + REG_MMU_INVLD_START_A);
writel_relaxed(iova + size - 1,
@@ -773,11 +773,12 @@ static const struct dev_pm_ops mtk_iommu_pm_ops = {
};
static const struct mtk_iommu_plat_data mt2712_data = {
- .m4u_plat = M4U_MT2712,
- .has_4gb_mode = true,
- .has_bclk = true,
- .has_vld_pa_rng = true,
- .larbid_remap = {0, 1, 2, 3, 4, 5, 6, 7, 8, 9},
+ .m4u_plat = M4U_MT2712,
+ .has_4gb_mode = true,
+ .has_bclk = true,
+ .has_vld_pa_rng = true,
+ .inv_sel_reg= REG_MMU_INV_SEL_GEN1,
+ .larbid_remap = {0, 1, 2, 3, 4, 5, 6, 7, 8, 9},
};
static const struct mtk_iommu_plat_data mt8173_data = {
@@ -785,12 +786,14 @@ static const struct mtk_iommu_plat_data mt8173_data = {
.has_4gb_mode = true,
.has_bclk = true,
.reset_axi= true,
+ .inv_sel_reg = REG_MMU_INV_SEL_GEN1,
.larbid_remap = {0, 1, 2, 3, 4, 5}, /* Linear mapping. */
};
static const struct mtk_iommu_plat_data mt8183_data = {
.m4u_plat = M4U_MT8183,
.reset_axi= true,
+ .inv_sel_reg = REG_MMU_INV_SEL_GEN1,
.larbid_remap = {0, 4, 5, 6, 7, 2, 3, 1},
};
diff --git a/drivers/iommu/mtk_iommu.h b/drivers/iommu/mtk_iommu.h
index d711ac630037..afd7a2de5c1e 100644
--- a/drivers/iommu/mtk_iommu.h
+++ b/drivers/iommu/mtk_iommu.h
@@ -43,6 +43,7 @@ struct mtk_iommu_plat_data {
boolhas_misc_ctrl;
boolhas_vld_pa_rng;
boolreset_axi;
+ u32 inv_sel_reg;
unsigned char larbid_remap[MTK_LARB_NR_MAX];
};
--
2.18.0
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v4 2/7] iommu/mediatek: Rename the register STANDARD_AXI_MODE(0x48) to MISC_CTRL
For iommu offset=0x48 register, only the previous mt8173/mt8183 use the
name STANDARD_AXI_MODE, all the latest SoC extend the register more
feature by different bits, for example: axi_mode, in_order_en, coherent_en
and so on. So rename REG_MMU_MISC_CTRL may be more proper.
This patch only rename the register name, no functional change.
Signed-off-by: Chao Hao
Reviewed-by: Yong Wu
---
drivers/iommu/mtk_iommu.c | 14 +++---
drivers/iommu/mtk_iommu.h | 2 +-
2 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
index 2be96f1cdbd2..88d3df5b91c2 100644
--- a/drivers/iommu/mtk_iommu.c
+++ b/drivers/iommu/mtk_iommu.c
@@ -41,7 +41,7 @@
#define F_INVLD_EN0BIT(0)
#define F_INVLD_EN1BIT(1)
-#define REG_MMU_STANDARD_AXI_MODE 0x048
+#define REG_MMU_MISC_CTRL 0x048
#define REG_MMU_DCM_DIS0x050
#define REG_MMU_CTRL_REG 0x110
@@ -573,8 +573,10 @@ static int mtk_iommu_hw_init(const struct mtk_iommu_data
*data)
}
writel_relaxed(0, data->base + REG_MMU_DCM_DIS);
- if (data->plat_data->reset_axi)
- writel_relaxed(0, data->base + REG_MMU_STANDARD_AXI_MODE);
+ if (data->plat_data->reset_axi) {
+ /* The register is called STANDARD_AXI_MODE in this case */
+ writel_relaxed(0, data->base + REG_MMU_MISC_CTRL);
+ }
if (devm_request_irq(data->dev, data->irq, mtk_iommu_isr, 0,
dev_name(data->dev), (void *)data)) {
@@ -718,8 +720,7 @@ static int __maybe_unused mtk_iommu_suspend(struct device
*dev)
struct mtk_iommu_suspend_reg *reg = &data->reg;
void __iomem *base = data->base;
- reg->standard_axi_mode = readl_relaxed(base +
- REG_MMU_STANDARD_AXI_MODE);
+ reg->misc_ctrl = readl_relaxed(base + REG_MMU_MISC_CTRL);
reg->dcm_dis = readl_relaxed(base + REG_MMU_DCM_DIS);
reg->ctrl_reg = readl_relaxed(base + REG_MMU_CTRL_REG);
reg->int_control0 = readl_relaxed(base + REG_MMU_INT_CONTROL0);
@@ -743,8 +744,7 @@ static int __maybe_unused mtk_iommu_resume(struct device
*dev)
dev_err(data->dev, "Failed to enable clk(%d) in resume\n", ret);
return ret;
}
- writel_relaxed(reg->standard_axi_mode,
- base + REG_MMU_STANDARD_AXI_MODE);
+ writel_relaxed(reg->misc_ctrl, base + REG_MMU_MISC_CTRL);
writel_relaxed(reg->dcm_dis, base + REG_MMU_DCM_DIS);
writel_relaxed(reg->ctrl_reg, base + REG_MMU_CTRL_REG);
writel_relaxed(reg->int_control0, base + REG_MMU_INT_CONTROL0);
diff --git a/drivers/iommu/mtk_iommu.h b/drivers/iommu/mtk_iommu.h
index ea949a324e33..1b6ea839b92c 100644
--- a/drivers/iommu/mtk_iommu.h
+++ b/drivers/iommu/mtk_iommu.h
@@ -18,7 +18,7 @@
#include
struct mtk_iommu_suspend_reg {
- u32 standard_axi_mode;
+ u32 misc_ctrl;
u32 dcm_dis;
u32 ctrl_reg;
u32 int_control0;
--
2.18.0
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v4 00/07] MT6779 IOMMU SUPPORT
This patchset adds mt6779 iommu support. mt6779 has two iommus, they are MM_IOMMU(M4U) and APU_IOMMU which used ARM Short-Descriptor translation format. The mt6779's MM_IOMMU-SMI and APU_IOMMU HW diagram is as below, it is only a brief diagram: EMI | -- || MM_IOMMUAPU_IOMMU || SMI_COMMOM--- APU_BUS || | SMI_LARB(0~11) | | || | || -- || | | | Multimedia engine CCU VPU MDLA EMDA All the connections are hardware fixed, software can not adjust it. Compared with mt8183, SMI_BUS_ID width has changed from 10 to 12. SMI Larb number is described in bit[11:7], Port number is described in bit[6:2]. In addition, there are some registers has changed in mt6779, so we need to redefine and reuse them. The patchset only used MM_IOMMU, so we only add MM_IOMMU basic function, such as smi_larb port definition, registers definition and hardware initialization. change notes: v4: 1. Rebase on v5.8-rc1. 2. Fix coding style. 3. Add F_MMU_IN_DRDER_WR_EN definition in MISC_CTRL to improve performance. v3: 1. Rebase on v5.7-rc1. 2. Remove unused port definition,ex:APU and CCU port in mt6779-larb-port.h. 3. Remove "change single domain to multiple domain" part(from PATCH v2 09/19 to PATCH v2 19/19). 4. Redesign mt6779 basic part (1)Add some register definition and reuse them. (2)Redesign smi larb bus ID to analyze IOMMU translation fault. (3)Only init MM_IOMMU and not use APU_IOMMU. http://lists.infradead.org/pipermail/linux-mediatek/2020-May/029811.html v2: 1. Rebase on v5.5-rc1. 2. Delete M4U_PORT_UNKNOWN define because of not use it. 3. Correct coding format. 4. Rename offset=0x48 register. 5. Split "iommu/mediatek: Add mt6779 IOMMU basic support(patch v1)" to several patches(patch v2). http://lists.infradead.org/pipermail/linux-mediatek/2020-January/026131.html v1: http://lists.infradead.org/pipermail/linux-mediatek/2019-November/024567.html Chao Hao (7): dt-bindings: mediatek: Add bindings for MT6779 iommu/mediatek: Rename the register STANDARD_AXI_MODE(0x48) to MISC_CTRL iommu/mediatek: Set MISC_CTRL register iommu/mediatek: Move inv_sel_reg into the plat_data iommu/mediatek: Add sub_comm id in translation fault iommu/mediatek: Add REG_MMU_WR_LEN definition preparing for mt6779 iommu/mediatek: Add mt6779 basic support .../bindings/iommu/mediatek,iommu.txt | 2 + drivers/iommu/mtk_iommu.c | 92 ++-- drivers/iommu/mtk_iommu.h | 10 +- include/dt-bindings/memory/mt6779-larb-port.h | 206 ++ 4 files changed, 285 insertions(+), 25 deletions(-) -- 2.18.0 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v4 3/7] iommu/mediatek: Set MISC_CTRL register
Add F_MMU_IN_ORDER_WR_EN definition in MISC_CTRL.
In order to improve performance, we always disable STANDARD_AXI_MODE
and IN_ORDER_WR_EN in MISC_CTRL.
Change since v3:
1. Rename Disable STANDARD_AXI_MODE in MISC_CTRL to Set MISC_CTRL register
2. Add F_MMU_IN_DRDER_WR_EN definition in MISC_CTRL
We need to disable in_order_write to improve performance
Cc: Yong Wu
Signed-off-by: Chao Hao
---
drivers/iommu/mtk_iommu.c | 11 +++
drivers/iommu/mtk_iommu.h | 1 +
2 files changed, 12 insertions(+)
diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
index 88d3df5b91c2..239d2cdbbc9f 100644
--- a/drivers/iommu/mtk_iommu.c
+++ b/drivers/iommu/mtk_iommu.c
@@ -42,6 +42,9 @@
#define F_INVLD_EN1BIT(1)
#define REG_MMU_MISC_CTRL 0x048
+#define F_MMU_IN_ORDER_WR_EN (BIT(1) | BIT(17))
+#define F_MMU_STANDARD_AXI_MODE_BIT(BIT(3) | BIT(19))
+
#define REG_MMU_DCM_DIS0x050
#define REG_MMU_CTRL_REG 0x110
@@ -578,6 +581,14 @@ static int mtk_iommu_hw_init(const struct mtk_iommu_data
*data)
writel_relaxed(0, data->base + REG_MMU_MISC_CTRL);
}
+ if (data->plat_data->has_misc_ctrl) {
+ /* For mm_iommu, it can improve performance by the setting */
+ regval = readl_relaxed(data->base + REG_MMU_MISC_CTRL);
+ regval &= ~F_MMU_STANDARD_AXI_MODE_BIT;
+ regval &= ~F_MMU_IN_ORDER_WR_EN;
+ writel_relaxed(regval, data->base + REG_MMU_MISC_CTRL);
+ }
+
if (devm_request_irq(data->dev, data->irq, mtk_iommu_isr, 0,
dev_name(data->dev), (void *)data)) {
writel_relaxed(0, data->base + REG_MMU_PT_BASE_ADDR);
diff --git a/drivers/iommu/mtk_iommu.h b/drivers/iommu/mtk_iommu.h
index 1b6ea839b92c..d711ac630037 100644
--- a/drivers/iommu/mtk_iommu.h
+++ b/drivers/iommu/mtk_iommu.h
@@ -40,6 +40,7 @@ struct mtk_iommu_plat_data {
/* HW will use the EMI clock if there isn't the "bclk". */
boolhas_bclk;
+ boolhas_misc_ctrl;
boolhas_vld_pa_rng;
boolreset_axi;
unsigned char larbid_remap[MTK_LARB_NR_MAX];
--
2.18.0
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 12/12] x86/traps: Fix up invalid PASID
Hi, Peter,
On Mon, Jun 15, 2020 at 09:09:28PM +0200, Peter Zijlstra wrote:
> On Mon, Jun 15, 2020 at 11:55:29AM -0700, Fenghua Yu wrote:
>
> > Or do you suggest to add a random new flag in struct thread_info instead
> > of a TIF flag?
>
> Why thread_info? What's wrong with something simple like the below. It
> takes a bit from the 'strictly current' flags word.
>
>
> diff --git a/include/linux/sched.h b/include/linux/sched.h
> index b62e6aaf28f0..fca830b97055 100644
> --- a/include/linux/sched.h
> +++ b/include/linux/sched.h
> @@ -801,6 +801,9 @@ struct task_struct {
> /* Stalled due to lack of memory */
> unsignedin_memstall:1;
> #endif
> +#ifdef CONFIG_PCI_PASID
> + unsignedhas_valid_pasid:1;
> +#endif
>
> unsigned long atomic_flags; /* Flags requiring atomic
> access. */
>
> diff --git a/kernel/fork.c b/kernel/fork.c
> index 142b23645d82..10b3891be99e 100644
> --- a/kernel/fork.c
> +++ b/kernel/fork.c
> @@ -955,6 +955,10 @@ static struct task_struct *dup_task_struct(struct
> task_struct *orig, int node)
> tsk->use_memdelay = 0;
> #endif
>
> +#ifdef CONFIG_PCI_PASID
> + tsk->has_valid_pasid = 0;
> +#endif
> +
> #ifdef CONFIG_MEMCG
> tsk->active_memcg = NULL;
> #endif
Can I add "Signed-off-by: Peter Zijlstra "
to this patch? I will send this patch in the next version of the series.
Thanks.
-Fenghua
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH] iommu/amd: Print extended features in one line to fix divergent log levels
Currently, Linux logs the two messages below.
[0.979142] pci :00:00.2: AMD-Vi: Extended features
(0xf77ef22294ada):
[0.979546] PPR NX GT IA GA PC GA_vAPIC
The log level of these lines differs though. The first one has level
*info*, while the second has level *warn*, which is confusing.
$ dmesg -T --level=info | grep "Extended features"
[Tue Jun 16 21:46:58 2020] pci :00:00.2: AMD-Vi: Extended features
(0xf77ef22294ada):
$ dmesg -T --level=warn | grep "PPR"
[Tue Jun 16 21:46:58 2020] PPR NX GT IA GA PC GA_vAPIC
The problem is, that commit 3928aa3f57 ("iommu/amd: Detect and enable
guest vAPIC support") introduced a newline, causing `pr_cont()`, used to
print the features, to default back to the default log level.
/**
* pr_cont - Continues a previous log message in the same line.
* @fmt: format string
* @...: arguments for the format string
*
* This macro expands to a printk with KERN_CONT loglevel. It should only be
* used when continuing a log message with no newline ('\n') enclosed.
Otherwise
* it defaults back to KERN_DEFAULT loglevel.
*/
#define pr_cont(fmt, ...) \
printk(KERN_CONT fmt, ##__VA_ARGS__)
So, remove the line break, so only one line is logged.
Fixes: 3928aa3f57 ("iommu/amd: Detect and enable guest vAPIC support")
Cc: Suravee Suthikulpanit
Cc: iommu@lists.linux-foundation.org
Signed-off-by: Paul Menzel
---
drivers/iommu/amd_iommu_init.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/iommu/amd_iommu_init.c b/drivers/iommu/amd_iommu_init.c
index 5b81fd16f5faf8..8d9b2c94178c43 100644
--- a/drivers/iommu/amd_iommu_init.c
+++ b/drivers/iommu/amd_iommu_init.c
@@ -1844,7 +1844,7 @@ static void print_iommu_info(void)
pci_info(pdev, "Found IOMMU cap 0x%hx\n", iommu->cap_ptr);
if (iommu->cap & (1 << IOMMU_CAP_EFR)) {
- pci_info(pdev, "Extended features (%#llx):\n",
+ pci_info(pdev, "Extended features (%#llx):",
iommu->features);
for (i = 0; i < ARRAY_SIZE(feat_str); ++i) {
if (iommu_feature(iommu, (1ULL << i)))
--
2.26.2
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC][PATCH 3/5] irqchip: Allow QCOM_PDC to be loadable as a perment module
On Tue, Jun 16, 2020 at 4:30 AM Maulik Shah wrote:
>
> Hi,
>
> On 6/16/2020 11:43 AM, John Stultz wrote:
> > Allows qcom-pdc driver to be loaded as a permenent module
>
> typo: permanent
>
> > Also, due to the fact that IRQCHIP_DECLARE becomes a no-op when
> > building as a module, we have to add the platform driver hooks
> > explicitly.
> >
> > Thanks to Saravana for his help on pointing out the
> > IRQCHIP_DECLARE issue and guidance on a solution.
> >
> > Cc: Andy Gross
> > Cc: Bjorn Andersson
> > Cc: Joerg Roedel
> > Cc: Thomas Gleixner
> > Cc: Jason Cooper
> > Cc: Marc Zyngier
> > Cc: Linus Walleij
> > Cc: Lina Iyer
> > Cc: Saravana Kannan
> > Cc: Todd Kjos
> > Cc: Greg Kroah-Hartman
> > Cc: linux-arm-...@vger.kernel.org
> > Cc: iommu@lists.linux-foundation.org
> > Cc: linux-g...@vger.kernel.org
> > Signed-off-by: John Stultz
> > ---
> > drivers/irqchip/Kconfig| 2 +-
> > drivers/irqchip/qcom-pdc.c | 30 ++
> > 2 files changed, 31 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
> > index 29fead208cad..12765bed08f9 100644
> > --- a/drivers/irqchip/Kconfig
> > +++ b/drivers/irqchip/Kconfig
> > @@ -425,7 +425,7 @@ config GOLDFISH_PIC
> >for Goldfish based virtual platforms.
> >
> > config QCOM_PDC
> > - bool "QCOM PDC"
> > + tristate "QCOM PDC"
> > depends on ARCH_QCOM
> > select IRQ_DOMAIN_HIERARCHY
> > help
> > diff --git a/drivers/irqchip/qcom-pdc.c b/drivers/irqchip/qcom-pdc.c
> > index 6ae9e1f0819d..98d74160afcd 100644
> > --- a/drivers/irqchip/qcom-pdc.c
> > +++ b/drivers/irqchip/qcom-pdc.c
> > @@ -11,7 +11,9 @@
> > #include
> > #include
> > #include
> > +#include
> > #include
> > +#include
> please move this include in order after of_device.h
> > #include
> > #include
> > #include
> > @@ -430,4 +432,32 @@ static int qcom_pdc_init(struct device_node *node,
> > struct device_node *parent)
> > return ret;
> > }
> >
> > +#ifdef MODULE
> > +static int qcom_pdc_probe(struct platform_device *pdev)
> > +{
> > + struct device_node *np = pdev->dev.of_node;
> > + struct device_node *parent = of_irq_find_parent(np);
> > +
> > + return qcom_pdc_init(np, parent);
> > +}
> > +
> > +static const struct of_device_id qcom_pdc_match_table[] = {
> > + { .compatible = "qcom,pdc" },
> > + {}
> > +};
> > +MODULE_DEVICE_TABLE(of, qcom_pdc_match_table);
> > +
> > +static struct platform_driver qcom_pdc_driver = {
> > + .probe = qcom_pdc_probe,
> > + .driver = {
> > + .name = "qcom-pdc",
> > + .of_match_table = qcom_pdc_match_table,
>
> can you please set .suppress_bind_attrs = true,
>
> This is to prevent bind/unbind using sysfs. Once irqchip driver module
> is loaded, it shouldn't get unbind at runtime.
Thanks, I really appreciate the review! I've made these changes on my
side and they'll be included in v2.
thanks
-john
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v5 08/12] device core: Introduce multiple dma pfn offsets
The new field in struct device 'dma_pfn_offset_map' is used to facilitate
the use of single or multiple pfn offsets between cpu addrs and dma addrs.
It subsumes the role of dev->dma_pfn_offset -- a uniform offset.
The function of_dma_get_range() has been modified to take two additional
arguments: the "map", which is an array that holds the information
regarding the pfn offset regions, and map_size, which is the size in bytes
of the map array.
of_dma_configure() is the typical manner to set pfn offsets but there are a
number of ad hoc assignments to dev->dma_pfn_offset in the kernel driver
code. These cases now invoke the function
dma_attach_uniform_pfn_offset(dev, pfn_offset).
Signed-off-by: Jim Quinlan
---
arch/arm/include/asm/dma-mapping.h| 9 +--
arch/arm/mach-keystone/keystone.c | 8 ++-
arch/sh/drivers/pci/pcie-sh7786.c | 3 +-
arch/sh/kernel/dma-coherent.c | 14 ++--
arch/x86/pci/sta2x11-fixup.c | 7 +-
drivers/acpi/arm64/iort.c | 4 +-
drivers/gpu/drm/sun4i/sun4i_backend.c | 5 +-
drivers/iommu/io-pgtable-arm.c| 2 +-
.../platform/sunxi/sun4i-csi/sun4i_csi.c | 5 +-
.../platform/sunxi/sun6i-csi/sun6i_csi.c | 4 +-
drivers/of/address.c | 71 ---
drivers/of/device.c | 19 +++--
drivers/of/of_private.h | 11 +--
drivers/of/unittest.c | 8 ++-
drivers/remoteproc/remoteproc_core.c | 2 +-
.../staging/media/sunxi/cedrus/cedrus_hw.c| 7 +-
drivers/usb/core/message.c| 4 +-
drivers/usb/core/usb.c| 2 +-
include/linux/device.h| 4 +-
include/linux/dma-direct.h| 14 +++-
include/linux/dma-mapping.h | 38 ++
kernel/dma/coherent.c | 11 +--
kernel/dma/mapping.c | 39 ++
23 files changed, 231 insertions(+), 60 deletions(-)
diff --git a/arch/arm/include/asm/dma-mapping.h
b/arch/arm/include/asm/dma-mapping.h
index bdd80ddbca34..f1e72f99468b 100644
--- a/arch/arm/include/asm/dma-mapping.h
+++ b/arch/arm/include/asm/dma-mapping.h
@@ -35,8 +35,9 @@ static inline const struct dma_map_ops
*get_arch_dma_ops(struct bus_type *bus)
#ifndef __arch_pfn_to_dma
static inline dma_addr_t pfn_to_dma(struct device *dev, unsigned long pfn)
{
- if (dev)
- pfn -= dev->dma_pfn_offset;
+ if (dev && dev->dma_pfn_offset_map)
+ pfn -= dma_pfn_offset_from_phys_addr(dev, PFN_PHYS(pfn));
+
return (dma_addr_t)__pfn_to_bus(pfn);
}
@@ -44,8 +45,8 @@ static inline unsigned long dma_to_pfn(struct device *dev,
dma_addr_t addr)
{
unsigned long pfn = __bus_to_pfn(addr);
- if (dev)
- pfn += dev->dma_pfn_offset;
+ if (dev && dev->dma_pfn_offset_map)
+ pfn += dma_pfn_offset_from_dma_addr(dev, addr);
return pfn;
}
diff --git a/arch/arm/mach-keystone/keystone.c
b/arch/arm/mach-keystone/keystone.c
index 638808c4e122..5890ec90599e 100644
--- a/arch/arm/mach-keystone/keystone.c
+++ b/arch/arm/mach-keystone/keystone.c
@@ -8,6 +8,7 @@
*/
#include
#include
+#include
#include
#include
#include
@@ -38,9 +39,10 @@ static int keystone_platform_notifier(struct notifier_block
*nb,
return NOTIFY_BAD;
if (!dev->of_node) {
- dev->dma_pfn_offset = keystone_dma_pfn_offset;
- dev_err(dev, "set dma_pfn_offset%08lx\n",
- dev->dma_pfn_offset);
+ int ret = dma_attach_uniform_pfn_offset(dev,
keystone_dma_pfn_offset);
+
+ dev_err(dev, "set dma_pfn_offset%08lx%s\n", dev->dma_pfn_offset,
+ ret ? " failed" : "");
}
return NOTIFY_OK;
}
diff --git a/arch/sh/drivers/pci/pcie-sh7786.c
b/arch/sh/drivers/pci/pcie-sh7786.c
index e0b568aaa701..3e63c6b6e070 100644
--- a/arch/sh/drivers/pci/pcie-sh7786.c
+++ b/arch/sh/drivers/pci/pcie-sh7786.c
@@ -12,6 +12,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -487,7 +488,7 @@ int pcibios_map_platform_irq(const struct pci_dev *pdev, u8
slot, u8 pin)
void pcibios_bus_add_device(struct pci_dev *pdev)
{
- pdev->dev.dma_pfn_offset = dma_pfn_offset;
+ dma_attach_uniform_pfn_offset(&pdev->dev, dma_pfn_offset);
}
static int __init sh7786_pcie_core_init(void)
diff --git a/arch/sh/kernel/dma-coherent.c b/arch/sh/kernel/dma-coherent.c
index d4811691b93c..f4a092e74910 100644
--- a/arch/sh/kernel/dma-coherent.c
+++ b/arch/sh/kernel/dma-coherent.c
@@ -14,6 +14,7 @@ void *arch_dma_alloc(struct device *dev, size_t size,
dma_addr_t *dma_handle,
{
void *ret, *ret_nocache;
int order = get_order(size);
+ phys_addr_t phys;
gfp |= __GFP_ZERO;
@@ -34,11 +35,12 @@ void *
[PATCH v5 00/12] PCI: brcmstb: enable PCIe for STB chips
Patchset Summary:
Enhance a PCIe host controller driver. Because of its unusual design
we are foced to change dev->dma_pfn_offset into a more general role
allowing multiple offsets.
v5:
Commit "device core: Introduce multiple dma pfn offsets"
-- in of/address.c: "map_size = 0" => "*map_size = 0"
-- use kcalloc instead of kzalloc (AndyS)
-- use PHYS_ADDR_MAX instead of "~(phys_addr_t)0"
Commit "PCI: brcmstb: Set internal memory viewport sizes"
-- now gives error on missing dma-ranges property.
Commit "dt-bindings: PCI: Add bindings for more Brcmstb chips"
-- removed "Allof:" from brcm,scb-sizes definition (RobH)
All Commits:
-- indentation style, use max chars 100 (AndyS)
-- rebased to torvalds master
v4:
Commit "device core: Introduce multiple dma pfn offsets"
-- of_dma_get_range() does not take a dev param but instead
takes two "out" params: map and map_size. We do this so
that the code that parses dma-ranges is separate from
the code that modifies 'dev'. (Nicolas)
-- the separate case of having a single pfn offset has
been removed and is now processed by going through the
map array. (Nicolas)
-- move attach_uniform_dma_pfn_offset() from of/address.c to
dma/mapping.c so that it does not depend on CONFIG_OF. (Nicolas)
-- devm_kcalloc => devm_kzalloc (DanC)
-- add/fix assignment to dev->dma_pfn_offset_map for func
attach_uniform_dma_pfn_offset() (DanC, Nicolas)
-- s/struct dma_pfn_offset_region/struct bus_dma_region/ (Nicolas)
-- s/attach_uniform_dma_pfn_offset/dma_attach_uniform_pfn_offset/
-- s/attach_dma_pfn_offset_map/dma_attach_pfn_offset_map/
-- More use of PFN_{PHYS,DOWN,UP}. (AndyS)
Commit "of: Include a dev param in of_dma_get_range()"
-- this commit was sqaushed with "device core: Introduce ..."
v3:
Commit "device core: Introduce multiple dma pfn offsets"
Commit "arm: dma-mapping: Invoke dma offset func if needed"
-- The above two commits have been squashed. More importantly,
the code has been modified so that the functionality for
multiple pfn offsets subsumes the use of dev->dma_pfn_offset.
In fact, dma_pfn_offset is removed and supplanted by
dma_pfn_offset_map, which is a pointer to an array. The
more common case of a uniform offset is now handled as
a map with a single entry, while cases requiring multiple
pfn offsets use a map with multiple entries. Code paths
that used to do this:
dev->dma_pfn_offset = mydrivers_pfn_offset;
have been changed to do this:
attach_uniform_dma_pfn_offset(dev, pfn_offset);
Commit "dt-bindings: PCI: Add bindings for more Brcmstb chips"
-- Add if/then clause for required props: resets, reset-names (RobH)
-- Change compatible list from const to enum (RobH)
-- Change list of u32-tuples to u64 (RobH)
Commit "of: Include a dev param in of_dma_get_range()"
-- modify of/unittests.c to add NULL param in of_dma_get_range() call.
Commit "device core: Add ability to handle multiple dma offsets"
-- align comment in device.h (AndyS).
-- s/cpu_beg/cpu_start/ and s/dma_beg/dma_start/ in struct
dma_pfn_offset_region (AndyS).
v2:
Commit: "device core: Add ability to handle multiple dma offsets"
o Added helper func attach_dma_pfn_offset_map() in address.c (Chistoph)
o Helpers funcs added to __phys_to_dma() & __dma_to_phys() (Christoph)
o Added warning when multiple offsets are needed and !DMA_PFN_OFFSET_MAP
o dev->dma_pfn_map => dev->dma_pfn_offset_map
o s/frm/from/ for dma_pfn_offset_frm_{phys,dma}_addr() (Christoph)
o In device.h: s/const void */const struct dma_pfn_offset_region */
o removed 'unlikely' from unlikely(dev->dma_pfn_offset_map) since
guarded by CONFIG_DMA_PFN_OFFSET_MAP (Christoph)
o Since dev->dma_pfn_offset is copied in usb/core/{usb,message}.c, now
dev->dma_pfn_offset_map is copied as well.
o Merged two of the DMA commits into one (Christoph).
Commit "arm: dma-mapping: Invoke dma offset func if needed":
o Use helper functions instead of #if CONFIG_DMA_PFN_OFFSET
Other commits' changes:
o Removed need for carrying of_id var in priv (Nicolas)
o Commit message rewordings (Bjorn)
o Commit log messages filled to 75 chars (Bjorn)
o devm_reset_control_get_shared())
=> devm_reset_control_get_optional_shared (Philipp)
o Add call to reset_control_assert() in PCIe remove routines (Philipp)
v1:
This patchset expands the usefulness of the Broadcom Settop Box PCIe
controller by building upon the PCIe driver used currently by the
Raspbery Pi. Other forms of this patchset were submitted by me years
ago and not accepted; the major sticking point was the code required
for the DMA remapping needed for the PCIe driver to work [1].
There have been many changes to the DMA and OF subsystems since that
time, making a cleaner and less intrusive patchset possible. This
patchset implements a generalization of "dev->dma_pfn_offset", except
that instead
Re: [RFC][PATCH 5/5] firmware: QCOM_SCM: Allow qcom_scm driver to be loadable as a permenent module
On Tue, Jun 16, 2020 at 12:55 AM Marc Zyngier wrote: > On 2020-06-16 07:13, John Stultz wrote: > > diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig > > index b510f67dfa49..714893535dd2 100644 > > --- a/drivers/iommu/Kconfig > > +++ b/drivers/iommu/Kconfig > > @@ -381,6 +381,7 @@ config SPAPR_TCE_IOMMU > > config ARM_SMMU > > tristate "ARM Ltd. System MMU (SMMU) Support" > > depends on (ARM64 || ARM || (COMPILE_TEST && !GENERIC_ATOMIC64)) && > > MMU > > + depends on QCOM_SCM || !QCOM_SCM #if QCOM_SCM=m this can't be =y > > This looks a bit ugly. Could you explain why we need this at the SMMU > level? I'd have expected the dependency to flow the other way around... Yea, so the arm-smmu-qcom.c file calls directly into the qcom-scm code via qcom_scm_qsmmu500_wait_safe_toggle() https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/iommu/arm-smmu-qcom.c?h=v5.8-rc1#n44 So if ARM_SMMU=y and QCOM_SCM=m we get: drivers/iommu/arm-smmu-qcom.o: In function `qcom_smmu500_reset': arm-smmu-qcom.c:(.text+0xb4): undefined reference to `qcom_scm_qsmmu500_wait_safe_toggle' Do you have a suggestion for an alternative approach? thanks -john ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 4/4] pci: export untrusted attribute in sysfs
On Tue, Jun 16, 2020 at 12:32 AM Christoph Hellwig wrote: > > On Mon, Jun 15, 2020 at 06:17:42PM -0700, Rajat Jain via iommu wrote: > > This is needed to allow the userspace to determine when an untrusted > > device has been added, and thus allowing it to bind the driver manually > > to it, if it so wishes. This is being done as part of the approach > > discussed at https://lkml.org/lkml/2020/6/9/1331 > > Please move the attribute to struct device instead of further > entrenching it in PCIe. Need clarification. The flag "untrusted" is currently a part of pci_dev struct, and is populated within the PCI subsystem. 1) Is your suggestion to move this flag as well as the attribute to device core (in "struct device")? This would allow other buses to populate/use this flag if they want. By default it'll be set to 0 for all devices (PCI subsystem will populate it based on platform info, like it does today). OR 2) Are you suggesting to keep the "untrusted" flag within PCI, but attach the sysfs attribute to the base device? (&pci_dev->dev)? Thanks, Rajat > I'm starting to grow tired of saying this > every other week while you guys are all moving into the entirely > wrong direction. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC][PATCH 3/5] irqchip: Allow QCOM_PDC to be loadable as a perment module
On Tue, Jun 16 2020 at 05:30 -0600, Maulik Shah wrote:
Hi,
On 6/16/2020 11:43 AM, John Stultz wrote:
Allows qcom-pdc driver to be loaded as a permenent module
typo: permanent
Also, due to the fact that IRQCHIP_DECLARE becomes a no-op when
building as a module, we have to add the platform driver hooks
explicitly.
Thanks to Saravana for his help on pointing out the
IRQCHIP_DECLARE issue and guidance on a solution.
Cc: Andy Gross
Cc: Bjorn Andersson
Cc: Joerg Roedel
Cc: Thomas Gleixner
Cc: Jason Cooper
Cc: Marc Zyngier
Cc: Linus Walleij
Cc: Lina Iyer
Cc: Saravana Kannan
Cc: Todd Kjos
Cc: Greg Kroah-Hartman
Cc: linux-arm-...@vger.kernel.org
Cc: iommu@lists.linux-foundation.org
Cc: linux-g...@vger.kernel.org
Signed-off-by: John Stultz
---
drivers/irqchip/Kconfig| 2 +-
drivers/irqchip/qcom-pdc.c | 30 ++
2 files changed, 31 insertions(+), 1 deletion(-)
diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
index 29fead208cad..12765bed08f9 100644
--- a/drivers/irqchip/Kconfig
+++ b/drivers/irqchip/Kconfig
@@ -425,7 +425,7 @@ config GOLDFISH_PIC
for Goldfish based virtual platforms.
config QCOM_PDC
- bool "QCOM PDC"
+ tristate "QCOM PDC"
depends on ARCH_QCOM
select IRQ_DOMAIN_HIERARCHY
help
diff --git a/drivers/irqchip/qcom-pdc.c b/drivers/irqchip/qcom-pdc.c
index 6ae9e1f0819d..98d74160afcd 100644
--- a/drivers/irqchip/qcom-pdc.c
+++ b/drivers/irqchip/qcom-pdc.c
@@ -11,7 +11,9 @@
#include
#include
#include
+#include
#include
+#include
please move this include in order after of_device.h
#include
#include
#include
@@ -430,4 +432,32 @@ static int qcom_pdc_init(struct device_node *node, struct
device_node *parent)
return ret;
}
+#ifdef MODULE
+static int qcom_pdc_probe(struct platform_device *pdev)
+{
+ struct device_node *np = pdev->dev.of_node;
+ struct device_node *parent = of_irq_find_parent(np);
+
+ return qcom_pdc_init(np, parent);
+}
+
+static const struct of_device_id qcom_pdc_match_table[] = {
+ { .compatible = "qcom,pdc" },
+ {}
+};
+MODULE_DEVICE_TABLE(of, qcom_pdc_match_table);
+
+static struct platform_driver qcom_pdc_driver = {
+ .probe = qcom_pdc_probe,
+ .driver = {
+ .name = "qcom-pdc",
+ .of_match_table = qcom_pdc_match_table,
can you please set .suppress_bind_attrs = true,
This is to prevent bind/unbind using sysfs. Once irqchip driver module
is loaded, it shouldn't get unbind at runtime.
That is a good point. We probably should do that to RPMH RSC driver as well.
Thanks,
Maulik
+ },
+};
+module_platform_driver(qcom_pdc_driver);
+#else
IRQCHIP_DECLARE(qcom_pdc, "qcom,pdc", qcom_pdc_init);
+#endif
+
+MODULE_DESCRIPTION("Qualcomm Technologies, Inc. Power Domain Controller");
+MODULE_LICENSE("GPL v2");
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of
Code Aurora Forum, hosted by The Linux Foundation
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 00/15] vfio: expose virtual Shared Virtual Addressing to VMs
On Tue, Jun 16, 2020 at 04:49:28PM +0100, Stefan Hajnoczi wrote: > On Tue, Jun 16, 2020 at 02:26:38AM +, Tian, Kevin wrote: > > > From: Stefan Hajnoczi > > > Sent: Monday, June 15, 2020 6:02 PM > > > > > > On Thu, Jun 11, 2020 at 05:15:19AM -0700, Liu Yi L wrote: > > > > Shared Virtual Addressing (SVA), a.k.a, Shared Virtual Memory (SVM) on > > > > Intel platforms allows address space sharing between device DMA and > > > > applications. SVA can reduce programming complexity and enhance > > > security. > > > > > > > > This VFIO series is intended to expose SVA usage to VMs. i.e. Sharing > > > > guest application address space with passthru devices. This is called > > > > vSVA in this series. The whole vSVA enabling requires QEMU/VFIO/IOMMU > > > > changes. For IOMMU and QEMU changes, they are in separate series (listed > > > > in the "Related series"). > > > > > > > > The high-level architecture for SVA virtualization is as below, the key > > > > design of vSVA support is to utilize the dual-stage IOMMU translation ( > > > > also known as IOMMU nesting translation) capability in host IOMMU. > > > > > > > > > > > > .-. .---. > > > > | vIOMMU| | Guest process CR3, FL only| > > > > | | '---' > > > > ./ > > > > | PASID Entry |--- PASID cache flush - > > > > '-' | > > > > | | V > > > > | |CR3 in GPA > > > > '-' > > > > Guest > > > > --| Shadow |--| > > > > vv v > > > > Host > > > > .-. .--. > > > > | pIOMMU| | Bind FL for GVA-GPA | > > > > | | '--' > > > > ./ | > > > > | PASID Entry | V (Nested xlate) > > > > '\.--. > > > > | | |SL for GPA-HPA, default domain| > > > > | | '--' > > > > '-' > > > > Where: > > > > - FL = First level/stage one page tables > > > > - SL = Second level/stage two page tables > > > > > > Hi, > > > Looks like an interesting feature! > > > > > > To check I understand this feature: can applications now pass virtual > > > addresses to devices instead of translating to IOVAs? > > > > > > If yes, can guest applications restrict the vSVA address space so the > > > device only has access to certain regions? > > > > > > On one hand replacing IOVA translation with virtual addresses simplifies > > > the application programming model, but does it give up isolation if the > > > device can now access all application memory? > > > > > > > with SVA each application is allocated with a unique PASID to tag its > > virtual address space. The device that claims SVA support must guarantee > > that one application can only program the device to access its own virtual > > address space (i.e. all DMAs triggered by this application are tagged with > > the application's PASID, and are translated by IOMMU's PASID-granular > > page table). So, isolation is not sacrificed in SVA. > > Isolation between applications is preserved but there is no isolation > between the device and the application itself. The application needs to > trust the device. Right. With all convenience comes security trust. With SVA there is an expectation that the device has the required security boundaries properly implemented. FWIW, what is our guarantee today that VF's are secure from one another or even its own PF? They can also generate transactions with any of its peer id's and there is nothing an IOMMU can do today. Other than rely on ACS. Even BusMaster enable can be ignored and devices (malicious or otherwise) can generate after the BM=0. With SVM you get the benefits of * Not having to register regions * Don't need to pin application space for DMA. > > Examples: > > 1. The device can snoop secret data from readable pages in the >application's virtual memory space. Aren't there other security technologies that can address this? > > 2. The device can gain arbitrary execution on the CPU by overwriting >control flow addresses (e.g. function pointers, stack return >addresses) in writable pages. I suppose technology like CET might be able to guard. The general expectation is code pages and anything that needs to be protected should be mapped nor writable. Cheers, Ashok ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 00/15] vfio: expose virtual Shared Virtual Addressing to VMs
On Tue, Jun 16, 2020 at 04:49:28PM +0100, Stefan Hajnoczi wrote: > Isolation between applications is preserved but there is no isolation > between the device and the application itself. The application needs to > trust the device. > > Examples: > > 1. The device can snoop secret data from readable pages in the >application's virtual memory space. > > 2. The device can gain arbitrary execution on the CPU by overwriting >control flow addresses (e.g. function pointers, stack return >addresses) in writable pages. To me, SVA seems to be that "middle layer" of secure where it's not as safe as VFIO_IOMMU_MAP_DMA which has buffer level granularity of control (but of course we pay overhead on buffer setups and on-the-fly translations), however it's far better than DMA with no IOMMU which can ruin the whole host/guest, because after all we do a lot of isolations as process based. IMHO it's the same as when we see a VM (or the QEMU process) as a whole along with the guest code. In some cases we don't care if the guest did some bad things to mess up with its own QEMU process. It is still ideal if we can even stop the guest from doing so, but when it's not easy to do it the ideal way, we just lower the requirement to not spread the influence to the host and other VMs. Thanks, -- Peter Xu ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 00/15] vfio: expose virtual Shared Virtual Addressing to VMs
On Tue, Jun 16, 2020 at 02:26:38AM +, Tian, Kevin wrote: > > From: Stefan Hajnoczi > > Sent: Monday, June 15, 2020 6:02 PM > > > > On Thu, Jun 11, 2020 at 05:15:19AM -0700, Liu Yi L wrote: > > > Shared Virtual Addressing (SVA), a.k.a, Shared Virtual Memory (SVM) on > > > Intel platforms allows address space sharing between device DMA and > > > applications. SVA can reduce programming complexity and enhance > > security. > > > > > > This VFIO series is intended to expose SVA usage to VMs. i.e. Sharing > > > guest application address space with passthru devices. This is called > > > vSVA in this series. The whole vSVA enabling requires QEMU/VFIO/IOMMU > > > changes. For IOMMU and QEMU changes, they are in separate series (listed > > > in the "Related series"). > > > > > > The high-level architecture for SVA virtualization is as below, the key > > > design of vSVA support is to utilize the dual-stage IOMMU translation ( > > > also known as IOMMU nesting translation) capability in host IOMMU. > > > > > > > > > .-. .---. > > > | vIOMMU| | Guest process CR3, FL only| > > > | | '---' > > > ./ > > > | PASID Entry |--- PASID cache flush - > > > '-' | > > > | | V > > > | |CR3 in GPA > > > '-' > > > Guest > > > --| Shadow |--| > > > vv v > > > Host > > > .-. .--. > > > | pIOMMU| | Bind FL for GVA-GPA | > > > | | '--' > > > ./ | > > > | PASID Entry | V (Nested xlate) > > > '\.--. > > > | | |SL for GPA-HPA, default domain| > > > | | '--' > > > '-' > > > Where: > > > - FL = First level/stage one page tables > > > - SL = Second level/stage two page tables > > > > Hi, > > Looks like an interesting feature! > > > > To check I understand this feature: can applications now pass virtual > > addresses to devices instead of translating to IOVAs? > > > > If yes, can guest applications restrict the vSVA address space so the > > device only has access to certain regions? > > > > On one hand replacing IOVA translation with virtual addresses simplifies > > the application programming model, but does it give up isolation if the > > device can now access all application memory? > > > > with SVA each application is allocated with a unique PASID to tag its > virtual address space. The device that claims SVA support must guarantee > that one application can only program the device to access its own virtual > address space (i.e. all DMAs triggered by this application are tagged with > the application's PASID, and are translated by IOMMU's PASID-granular > page table). So, isolation is not sacrificed in SVA. Isolation between applications is preserved but there is no isolation between the device and the application itself. The application needs to trust the device. Examples: 1. The device can snoop secret data from readable pages in the application's virtual memory space. 2. The device can gain arbitrary execution on the CPU by overwriting control flow addresses (e.g. function pointers, stack return addresses) in writable pages. Stefan signature.asc Description: PGP signature ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 00/15] vfio: expose virtual Shared Virtual Addressing to VMs
On Mon, Jun 15, 2020 at 12:39:40PM +, Liu, Yi L wrote: > > From: Stefan Hajnoczi > > Sent: Monday, June 15, 2020 6:02 PM > > > > On Thu, Jun 11, 2020 at 05:15:19AM -0700, Liu Yi L wrote: > > > Shared Virtual Addressing (SVA), a.k.a, Shared Virtual Memory (SVM) on > > > Intel platforms allows address space sharing between device DMA and > > > applications. SVA can reduce programming complexity and enhance security. > > > > > > This VFIO series is intended to expose SVA usage to VMs. i.e. Sharing > > > guest application address space with passthru devices. This is called > > > vSVA in this series. The whole vSVA enabling requires QEMU/VFIO/IOMMU > > > changes. For IOMMU and QEMU changes, they are in separate series (listed > > > in the "Related series"). > > > > > > The high-level architecture for SVA virtualization is as below, the key > > > design of vSVA support is to utilize the dual-stage IOMMU translation ( > > > also known as IOMMU nesting translation) capability in host IOMMU. > > > > > > > > > .-. .---. > > > | vIOMMU| | Guest process CR3, FL only| > > > | | '---' > > > ./ > > > | PASID Entry |--- PASID cache flush - > > > '-' | > > > | | V > > > | |CR3 in GPA > > > '-' > > > Guest > > > --| Shadow |--| > > > vv v > > > Host > > > .-. .--. > > > | pIOMMU| | Bind FL for GVA-GPA | > > > | | '--' > > > ./ | > > > | PASID Entry | V (Nested xlate) > > > '\.--. > > > | | |SL for GPA-HPA, default domain| > > > | | '--' > > > '-' > > > Where: > > > - FL = First level/stage one page tables > > > - SL = Second level/stage two page tables > > > > Hi, > > Looks like an interesting feature! > > thanks for the interest. Stefan :-) > > > To check I understand this feature: can applications now pass virtual > > addresses to devices instead of translating to IOVAs? > > yes, application could pass virtual addresses to device directly. As > long as the virtual address is mapped in cpu page table, then IOMMU > would get it translated to physical address. > > > If yes, can guest applications restrict the vSVA address space so the > > device only has access to certain regions? > > do you mean restrict the access of certain virtual address regions of > guest application ? or certain guest memory? :-) Your reply below answered my question. I was wondering if applications can protect parts of their virtual memory space that should not be accessed by the device. It makes sense that there is a trade-off to simplify the programming model and performance might also be better if the application doesn't need to DMA map/unmap buffers frequently. Stefan signature.asc Description: PGP signature ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 1/3] docs: IOMMU user API
On Thu, 11 Jun 2020 17:27:27 -0700 Jacob Pan wrote: > > > > But then I thought it even better if VFIO leaves the entire > > copy_from_user() to the layer consuming it. > > > OK. Sounds good, that was what Kevin suggested also. I just wasn't > sure how much VFIO wants to inspect, I thought VFIO layer wanted to > do a sanity check. > > Anyway, I will move copy_from_user to iommu uapi layer. Just one more point brought up by Yi when we discuss this offline. If we move copy_from_user to iommu uapi layer, then there will be multiple copy_from_user calls for the same data when a VFIO container has multiple domains, devices. For bind, it might be OK. But might be additional overhead for TLB flush request from the guest. Thoughts? Jacob ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 08/12] mm: Define pasid in mm
Hi, Jean,
On Tue, Jun 16, 2020 at 10:28:19AM +0200, Jean-Philippe Brucker wrote:
> On Fri, Jun 12, 2020 at 05:41:29PM -0700, Fenghua Yu wrote:
> > diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
> > index 64ede5f150dc..5778db3aa42d 100644
> > --- a/include/linux/mm_types.h
> > +++ b/include/linux/mm_types.h
> > @@ -538,6 +538,10 @@ struct mm_struct {
> > atomic_long_t hugetlb_usage;
> > #endif
> > struct work_struct async_put_work;
> > +
> > +#ifdef CONFIG_PCI_PASID
>
> Non-PCI devices can also use a PASID (e.g. Arm's SubstreamID). How about
> CONFIG_IOMMU_SUPPORT?
Sure. I will change it to CONFIG_IOMMU_SUPPORT.
Thanks.
-Fenghua
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH] iommu: Allow page responses without PASID
Some PCIe devices do not expect a PASID value in PRI Page Responses.
If the "PRG Response PASID Required" bit in the PRI capability is zero,
then the OS should not set the PASID field. Similarly on Arm SMMU,
responses to stall events do not have a PASID.
Currently iommu_page_response() systematically checks that the PASID in
the page response corresponds to the one in the page request. This can't
work with virtualization because a page response coming from a guest OS
won't have a PASID if the passed-through device does not require one.
Add a flag to page requests that declares whether the corresponding
response needs to have a PASID. When this flag isn't set, allow page
responses without PASID.
Reported-by: Shameerali Kolothum Thodi
Signed-off-by: Jean-Philippe Brucker
---
include/uapi/linux/iommu.h | 6 +-
drivers/iommu/iommu.c | 23 +--
2 files changed, 22 insertions(+), 7 deletions(-)
diff --git a/include/uapi/linux/iommu.h b/include/uapi/linux/iommu.h
index e907b7091a463..c2b2caf9ed412 100644
--- a/include/uapi/linux/iommu.h
+++ b/include/uapi/linux/iommu.h
@@ -81,7 +81,10 @@ struct iommu_fault_unrecoverable {
/**
* struct iommu_fault_page_request - Page Request data
* @flags: encodes whether the corresponding fields are valid and whether this
- * is the last page in group (IOMMU_FAULT_PAGE_REQUEST_* values)
+ * is the last page in group (IOMMU_FAULT_PAGE_REQUEST_* values).
+ * When IOMMU_FAULT_PAGE_RESPONSE_NEEDS_PASID is set, the page response
+ * must have the same PASID value as the page request. When it is
clear,
+ * the page response should not have a PASID.
* @pasid: Process Address Space ID
* @grpid: Page Request Group Index
* @perm: requested page permissions (IOMMU_FAULT_PERM_* values)
@@ -92,6 +95,7 @@ struct iommu_fault_page_request {
#define IOMMU_FAULT_PAGE_REQUEST_PASID_VALID (1 << 0)
#define IOMMU_FAULT_PAGE_REQUEST_LAST_PAGE (1 << 1)
#define IOMMU_FAULT_PAGE_REQUEST_PRIV_DATA (1 << 2)
+#define IOMMU_FAULT_PAGE_RESPONSE_NEEDS_PASID (1 << 3)
__u32 flags;
__u32 pasid;
__u32 grpid;
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index d43120eb1dc56..1ed1e14a1f0cf 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -1185,11 +1185,12 @@ EXPORT_SYMBOL_GPL(iommu_report_device_fault);
int iommu_page_response(struct device *dev,
struct iommu_page_response *msg)
{
- bool pasid_valid;
+ bool needs_pasid;
int ret = -EINVAL;
struct iommu_fault_event *evt;
struct iommu_fault_page_request *prm;
struct dev_iommu *param = dev->iommu;
+ bool has_pasid = msg->flags & IOMMU_PAGE_RESP_PASID_VALID;
struct iommu_domain *domain = iommu_get_domain_for_dev(dev);
if (!domain || !domain->ops->page_response)
@@ -1214,14 +1215,24 @@ int iommu_page_response(struct device *dev,
*/
list_for_each_entry(evt, ¶m->fault_param->faults, list) {
prm = &evt->fault.prm;
- pasid_valid = prm->flags & IOMMU_FAULT_PAGE_REQUEST_PASID_VALID;
+ if (prm->grpid != msg->grpid)
+ continue;
- if ((pasid_valid && prm->pasid != msg->pasid) ||
- prm->grpid != msg->grpid)
+ /*
+* If the PASID is required, the corresponding request is
+* matched using the group ID, the PASID valid bit and the PASID
+* value. Otherwise only the group ID matches request and
+* response.
+*/
+ needs_pasid = prm->flags &
IOMMU_FAULT_PAGE_RESPONSE_NEEDS_PASID;
+ if (needs_pasid && (!has_pasid || msg->pasid != prm->pasid))
continue;
- /* Sanitize the reply */
- msg->flags = pasid_valid ? IOMMU_PAGE_RESP_PASID_VALID : 0;
+ if (!needs_pasid && has_pasid) {
+ /* No big deal, just clear it. */
+ msg->flags &= ~IOMMU_PAGE_RESP_PASID_VALID;
+ msg->pasid = 0;
+ }
ret = domain->ops->page_response(dev, evt, msg);
list_del(&evt->list);
--
2.27.0
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC][PATCH 3/5] irqchip: Allow QCOM_PDC to be loadable as a perment module
Hi,
On 6/16/2020 11:43 AM, John Stultz wrote:
Allows qcom-pdc driver to be loaded as a permenent module
typo: permanent
Also, due to the fact that IRQCHIP_DECLARE becomes a no-op when
building as a module, we have to add the platform driver hooks
explicitly.
Thanks to Saravana for his help on pointing out the
IRQCHIP_DECLARE issue and guidance on a solution.
Cc: Andy Gross
Cc: Bjorn Andersson
Cc: Joerg Roedel
Cc: Thomas Gleixner
Cc: Jason Cooper
Cc: Marc Zyngier
Cc: Linus Walleij
Cc: Lina Iyer
Cc: Saravana Kannan
Cc: Todd Kjos
Cc: Greg Kroah-Hartman
Cc: linux-arm-...@vger.kernel.org
Cc: iommu@lists.linux-foundation.org
Cc: linux-g...@vger.kernel.org
Signed-off-by: John Stultz
---
drivers/irqchip/Kconfig| 2 +-
drivers/irqchip/qcom-pdc.c | 30 ++
2 files changed, 31 insertions(+), 1 deletion(-)
diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
index 29fead208cad..12765bed08f9 100644
--- a/drivers/irqchip/Kconfig
+++ b/drivers/irqchip/Kconfig
@@ -425,7 +425,7 @@ config GOLDFISH_PIC
for Goldfish based virtual platforms.
config QCOM_PDC
- bool "QCOM PDC"
+ tristate "QCOM PDC"
depends on ARCH_QCOM
select IRQ_DOMAIN_HIERARCHY
help
diff --git a/drivers/irqchip/qcom-pdc.c b/drivers/irqchip/qcom-pdc.c
index 6ae9e1f0819d..98d74160afcd 100644
--- a/drivers/irqchip/qcom-pdc.c
+++ b/drivers/irqchip/qcom-pdc.c
@@ -11,7 +11,9 @@
#include
#include
#include
+#include
#include
+#include
please move this include in order after of_device.h
#include
#include
#include
@@ -430,4 +432,32 @@ static int qcom_pdc_init(struct device_node *node, struct
device_node *parent)
return ret;
}
+#ifdef MODULE
+static int qcom_pdc_probe(struct platform_device *pdev)
+{
+ struct device_node *np = pdev->dev.of_node;
+ struct device_node *parent = of_irq_find_parent(np);
+
+ return qcom_pdc_init(np, parent);
+}
+
+static const struct of_device_id qcom_pdc_match_table[] = {
+ { .compatible = "qcom,pdc" },
+ {}
+};
+MODULE_DEVICE_TABLE(of, qcom_pdc_match_table);
+
+static struct platform_driver qcom_pdc_driver = {
+ .probe = qcom_pdc_probe,
+ .driver = {
+ .name = "qcom-pdc",
+ .of_match_table = qcom_pdc_match_table,
can you please set .suppress_bind_attrs = true,
This is to prevent bind/unbind using sysfs. Once irqchip driver module
is loaded, it shouldn't get unbind at runtime.
Thanks,
Maulik
+ },
+};
+module_platform_driver(qcom_pdc_driver);
+#else
IRQCHIP_DECLARE(qcom_pdc, "qcom,pdc", qcom_pdc_init);
+#endif
+
+MODULE_DESCRIPTION("Qualcomm Technologies, Inc. Power Domain Controller");
+MODULE_LICENSE("GPL v2");
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of
Code Aurora Forum, hosted by The Linux Foundation
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH RFC v2 4/4] iommu/arm-smmu-v3: Remove cmpxchg() in arm_smmu_cmdq_issue_cmdlist()
It has been shown that the cmpxchg() for finding space in the cmdq can
be a real bottleneck:
- for more CPUs contending the cmdq, the cmpxchg() will fail more often
- since the software-maintained cons pointer is updated on the same 64b
memory region, the chance of cmpxchg() failure increases again
The cmpxchg() is removed as part of 2 related changes:
- Update prod and cmdq owner in a single atomic add operation. For this, we
count the prod and owner in separate regions in prod memory.
As with simple binary counting, once the prod+wrap fields overflow, they
will zero. They should never overflow into "owner" region, and we zero
the non-owner, prod region for each owner. This maintains the prod
pointer.
As for the "owner", we now count this value, instead of setting a flag.
Similar to before, once the owner has finished gathering, it will clear
a mask. As such, a CPU declares itself as the "owner" when it reads zero
for this region. This zeroing will also clear possible overflow in
wrap+prod region, above.
The owner is now responsible for cmdq locking to avoid possible deadlock.
The owner will lock the cmdq for all non-owers it has gathered when they
have space in the queue and have written their entries.
- Check for space in the cmdq after the prod pointer has been assigned.
We don't bother checking for space in the cmdq before assigning the prod
pointer, as this would be racy.
So since the prod pointer is updated unconditionally, it would be common
for no space to be available in the cmdq when prod is assigned - that
is, according the software-maintained prod and cons pointer. So now
it must be ensured that the entries are not yet written and not until
there is space.
How the prod pointer is maintained also leads to a strange condition
where the prod pointer can wrap past the cons pointer. We can detect this
condition, and report no space here. However, a prod pointer progressed
twice past the cons pointer cannot be detected. But it can be ensured that
this that this scenario does not occur, as we limit the amount of
commands any CPU can issue at any given time, such that we cannot
progress prod pointer further.
Signed-off-by: John Garry
---
drivers/iommu/arm-smmu-v3.c | 102 +---
1 file changed, 61 insertions(+), 41 deletions(-)
diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index 36648163364c..fd42a98f501f 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -772,10 +772,19 @@ static bool queue_has_space(struct arm_smmu_ll_queue *q,
u32 n)
prod = Q_IDX(q, q->prod);
cons = Q_IDX(q, q->cons);
- if (Q_WRP(q, q->prod) == Q_WRP(q, q->cons))
+ if (Q_WRP(q, q->prod) == Q_WRP(q, q->cons)) {
+ /* check if we have wrapped twice, meaning definitely no space
*/
+ if (cons > prod)
+ return false;
+
space = (1 << q->max_n_shift) - (prod - cons);
- else
+ } else {
+ /* similar check to above */
+ if (prod > cons)
+ return false;
+
space = cons - prod;
+ }
return space >= n;
}
@@ -1073,7 +1082,7 @@ static void arm_smmu_cmdq_skip_err(struct arm_smmu_device
*smmu)
* fails if the caller appears to be the last lock holder (yes, this is
* racy). All successful UNLOCK routines have RELEASE semantics.
*/
-static void arm_smmu_cmdq_shared_lock(struct arm_smmu_cmdq *cmdq)
+static void arm_smmu_cmdq_shared_lock(struct arm_smmu_cmdq *cmdq, int count)
{
int val;
@@ -1083,12 +1092,12 @@ static void arm_smmu_cmdq_shared_lock(struct
arm_smmu_cmdq *cmdq)
* to INT_MIN so these increments won't hurt as the value will remain
* negative.
*/
- if (atomic_fetch_inc_relaxed(&cmdq->lock) >= 0)
+ if (atomic_fetch_add_relaxed(count, &cmdq->lock) >= 0)
return;
do {
val = atomic_cond_read_relaxed(&cmdq->lock, VAL >= 0);
- } while (atomic_cmpxchg_relaxed(&cmdq->lock, val, val + 1) != val);
+ } while (atomic_cmpxchg_relaxed(&cmdq->lock, val, val + count) != val);
}
static void arm_smmu_cmdq_shared_unlock(struct arm_smmu_cmdq *cmdq)
@@ -1374,8 +1383,10 @@ static void arm_smmu_cmdq_write_entries(struct
arm_smmu_cmdq *cmdq, u64 *cmds,
* insert their own list of commands then all of the commands from one
* CPU will appear before any of the commands from the other CPU.
*
- * - A CMD_SYNC is always inserted, ensuring that any CPU does not issue
- * more than the permitted amount commands at once.
+ * - A CMD_SYNC is always inserted, which ensures we limit the prod pointer
+ * for when the cmdq is full, such that we don't wrap more than twice.
+ * It also makes it easy for the owner to know by how many to increment the
+ * cmdq lock.
*/
static int arm_smmu_cmdq_issue_cmdlist(stru
[PATCH RFC v2 2/4] iommu/arm-smmu-v3: Calculate bits for prod and owner
Since the arm_smmu_ll_queue.prod will be for counting the "owner" value
and also HW prod pointer, calculate how many bits are available for and
used by each.
This is based on the number of possible CPUs in the system. And we require
that each CPU can issue a minimum of 2 commands per batch - 1 x CMD_SYNC
and at least 1 x other.
Ignoring limits of HW max_n_shift and HW cmdq memory allocation, approx 16K
is the max supported CPUs. For this, max_n_shift would be 14.
Signed-off-by: John Garry
---
drivers/iommu/arm-smmu-v3.c | 41 -
1 file changed, 40 insertions(+), 1 deletion(-)
diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index a8e814c652fe..c3562dc35d45 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -532,6 +532,8 @@ struct arm_smmu_ll_queue {
u8 __pad[SMP_CACHE_BYTES];
} cacheline_aligned_in_smp;
u32 max_n_shift;
+ u32 max_cmd_per_batch;
+ u32 owner_count_shift;
};
struct arm_smmu_queue {
@@ -1515,7 +1517,10 @@ static void arm_smmu_cmdq_batch_add(struct
arm_smmu_device *smmu,
struct arm_smmu_cmdq_batch *cmds,
struct arm_smmu_cmdq_ent *cmd)
{
- if (cmds->num == CMDQ_BATCH_ENTRIES) {
+ struct arm_smmu_cmdq *q = &smmu->cmdq;
+ struct arm_smmu_ll_queue *llq = &q->q.llq;
+
+ if (cmds->num == llq->max_cmd_per_batch) {
arm_smmu_cmdq_issue_cmdlist(smmu, cmds->cmds, cmds->num, false);
cmds->num = 0;
}
@@ -3141,8 +3146,26 @@ static int arm_smmu_init_one_queue(struct
arm_smmu_device *smmu,
unsigned long cons_off,
size_t dwords, const char *name)
{
+ int cpus = num_possible_cpus();
size_t qsz;
+ /*
+* We can get the number of bits required for owner counting by
+* log2(nr possible cpus) + 1
+*/
+ int bits_for_cmdq_owner = ilog2(cpus) + 1;
+ /*
+* 1-bit for overflow, 1-bit for wrap, 1-bit extra to ensure prod+wrap
+* does not overflow into CPU count.
+*/
+ int bits_available_for_prod = 32 - 3 - bits_for_cmdq_owner;
+ int entries_for_prod;
+
+ if (bits_available_for_prod < 1) /* How many CPUs??? */
+ return -ENOMEM;
+
+ q->llq.max_n_shift = min_t(int, q->llq.max_n_shift,
+ bits_available_for_prod);
do {
qsz = ((1 << q->llq.max_n_shift) * dwords) << 3;
q->base = dmam_alloc_coherent(smmu->dev, qsz, &q->base_dma,
@@ -3152,6 +3175,22 @@ static int arm_smmu_init_one_queue(struct
arm_smmu_device *smmu,
q->llq.max_n_shift--;
} while (1);
+ entries_for_prod = 1 << q->llq.max_n_shift;
+
+ /*
+* We need at least 2 commands in a batch (1 x CMD_SYNC and 1 x
+* whatever else).
+*/
+ if (entries_for_prod < 2 * cpus)
+ return -ENOMEM;
+
+ /*
+* When finding max_cmd_per_batch, deduct 1 entry per batch to take
+* account CMD_SYNC
+*/
+ q->llq.max_cmd_per_batch = min_t(u32, (entries_for_prod - cpus) / cpus,
+CMDQ_BATCH_ENTRIES);
+ q->llq.owner_count_shift = q->llq.max_n_shift + 2;
if (!q->base) {
dev_err(smmu->dev,
--
2.26.2
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v2 1/4] iommu/arm-smmu-v3: Fix trivial typo
Set "cmq" -> "cmdq".
Signed-off-by: John Garry
---
drivers/iommu/arm-smmu-v3.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index f578677a5c41..a8e814c652fe 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -1479,7 +1479,7 @@ static int arm_smmu_cmdq_issue_cmdlist(struct
arm_smmu_device *smmu,
}
/*
-* Try to unlock the cmq lock. This will fail if we're the last
+* Try to unlock the cmdq lock. This will fail if we're the last
* reader, in which case we can safely update cmdq->q.llq.cons
*/
if (!arm_smmu_cmdq_shared_tryunlock(cmdq)) {
--
2.26.2
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH RFC v2 0/4] iommu/arm-smmu-v3: Improve cmdq lock efficiency
As mentioned in [0], the CPU may consume many cycles processing arm_smmu_cmdq_issue_cmdlist(). One issue we find is the cmpxchg() loop to get space on the queue takes approx 25% of the cycles for this function. This series removes that cmpxchg(). For my NVMe test with 3x NVMe SSDs, I'm getting a ~24% throughput increase: Before: 1310 IOPs After: 1630 IOPs I also have a test harness to check the rate of DMA map+unmaps we can achieve: CPU count 32 64 128 Before: 63187 19418 10169 After: 93287 44789 15862 (unit is map+unmaps per CPU per second) There's no specific problem that I know of with this series, as previous issues should now be fixed - but I'm a bit nervous about how we deal with the queue being full and wrapping. And I want to test more. Thanks [0] https://lore.kernel.org/linux-iommu/b926444035e5e2439431908e3842afd24b8...@dggemi525-mbs.china.huawei.com/T/#ma02e301c38c3e94b7725e685757c27e39c7cbde3 John Garry (4): iommu/arm-smmu-v3: Fix trivial typo iommu/arm-smmu-v3: Calculate bits for prod and owner iommu/arm-smmu-v3: Always issue a CMD_SYNC per batch iommu/arm-smmu-v3: Remove cmpxchg() in arm_smmu_cmdq_issue_cmdlist() drivers/iommu/arm-smmu-v3.c | 210 ++-- 1 file changed, 131 insertions(+), 79 deletions(-) -- 2.26.2 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH RFC v2 3/4] iommu/arm-smmu-v3: Always issue a CMD_SYNC per batch
To ensure that a CPU does not send more than a permitted amount of commands
to the cmdq, ensure that each batch includes a CMD_SYNC. When issuing a
CMD_SYNC, we always wait for the consumption of its batch of commands - as
such, we guarantee that any CPU will not issue more than its permitted
amount.
Signed-off-by: John Garry
---
drivers/iommu/arm-smmu-v3.c | 87 +
1 file changed, 40 insertions(+), 47 deletions(-)
diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index c3562dc35d45..36648163364c 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -1373,11 +1373,15 @@ static void arm_smmu_cmdq_write_entries(struct
arm_smmu_cmdq *cmdq, u64 *cmds,
* - Command insertion is totally ordered, so if two CPUs each race to
* insert their own list of commands then all of the commands from one
* CPU will appear before any of the commands from the other CPU.
+ *
+ * - A CMD_SYNC is always inserted, ensuring that any CPU does not issue
+ * more than the permitted amount commands at once.
*/
static int arm_smmu_cmdq_issue_cmdlist(struct arm_smmu_device *smmu,
- u64 *cmds, int n, bool sync)
+ u64 *cmds, int n)
{
u64 cmd_sync[CMDQ_ENT_DWORDS];
+ const int sync = 1;
u32 prod;
unsigned long flags;
bool owner;
@@ -1419,19 +1423,18 @@ static int arm_smmu_cmdq_issue_cmdlist(struct
arm_smmu_device *smmu,
* Dependency ordering from the cmpxchg() loop above.
*/
arm_smmu_cmdq_write_entries(cmdq, cmds, llq.prod, n);
- if (sync) {
- prod = queue_inc_prod_n(&llq, n);
- arm_smmu_cmdq_build_sync_cmd(cmd_sync, smmu, prod);
- queue_write(Q_ENT(&cmdq->q, prod), cmd_sync, CMDQ_ENT_DWORDS);
- /*
-* In order to determine completion of our CMD_SYNC, we must
-* ensure that the queue can't wrap twice without us noticing.
-* We achieve that by taking the cmdq lock as shared before
-* marking our slot as valid.
-*/
- arm_smmu_cmdq_shared_lock(cmdq);
- }
+ prod = queue_inc_prod_n(&llq, n);
+ arm_smmu_cmdq_build_sync_cmd(cmd_sync, smmu, prod);
+ queue_write(Q_ENT(&cmdq->q, prod), cmd_sync, CMDQ_ENT_DWORDS);
+
+ /*
+* In order to determine completion of our CMD_SYNC, we must
+* ensure that the queue can't wrap twice without us noticing.
+* We achieve that by taking the cmdq lock as shared before
+* marking our slot as valid.
+*/
+ arm_smmu_cmdq_shared_lock(cmdq);
/* 3. Mark our slots as valid, ensuring commands are visible first */
dma_wmb();
@@ -1468,26 +1471,22 @@ static int arm_smmu_cmdq_issue_cmdlist(struct
arm_smmu_device *smmu,
atomic_set_release(&cmdq->owner_prod, prod);
}
- /* 5. If we are inserting a CMD_SYNC, we must wait for it to complete */
- if (sync) {
- llq.prod = queue_inc_prod_n(&llq, n);
- ret = arm_smmu_cmdq_poll_until_sync(smmu, &llq);
- if (ret) {
- dev_err_ratelimited(smmu->dev,
- "CMD_SYNC timeout at 0x%08x [hwprod
0x%08x, hwcons 0x%08x]\n",
- llq.prod,
- readl_relaxed(cmdq->q.prod_reg),
- readl_relaxed(cmdq->q.cons_reg));
- }
+ /* 5. Since we always insert a CMD_SYNC, we must wait for it to
complete */
+ llq.prod = queue_inc_prod_n(&llq, n);
+ ret = arm_smmu_cmdq_poll_until_sync(smmu, &llq);
+ if (ret) {
+ dev_err_ratelimited(smmu->dev, "CMD_SYNC timeout at 0x%08x
[hwprod 0x%08x, hwcons 0x%08x]\n",
+ llq.prod, readl_relaxed(cmdq->q.prod_reg),
+ readl_relaxed(cmdq->q.cons_reg));
+ }
- /*
-* Try to unlock the cmdq lock. This will fail if we're the last
-* reader, in which case we can safely update cmdq->q.llq.cons
-*/
- if (!arm_smmu_cmdq_shared_tryunlock(cmdq)) {
- WRITE_ONCE(cmdq->q.llq.cons, llq.cons);
- arm_smmu_cmdq_shared_unlock(cmdq);
- }
+ /*
+* Try to unlock the cmdq lock. This will fail if we're the last reader,
+* in which case we can safely update cmdq->q.llq.cons
+*/
+ if (!arm_smmu_cmdq_shared_tryunlock(cmdq)) {
+ WRITE_ONCE(cmdq->q.llq.cons, llq.cons);
+ arm_smmu_cmdq_shared_unlock(cmdq);
}
local_irq_restore(flags);
@@ -1505,12 +1504,7 @@ static int arm_smmu_cmdq_issue_cmd(struct
arm_smmu_device *smmu,
re
Re: [PATCH] dma-direct: enable mmap for !CONFIG_MMU
On 6/15/20 8:05 AM, Christoph Hellwig wrote:
> nommu configfs can trivially map the coherent allocations to user space,
> as no actual page table setup is required and the kernel and the user
> space programs share the same address space.
>
> Fixes: 62fcee9a3bd7 ("dma-mapping: remove CONFIG_ARCH_NO_COHERENT_DMA_MMAP")
> Signed-off-by: Christoph Hellwig
> Reported-by: dillon min
> Reviewed-by: Vladimir Murzin
> Tested-by: dillon min
> ---
> kernel/dma/Kconfig | 1 +
> kernel/dma/direct.c | 14 --
> 2 files changed, 1 insertion(+), 14 deletions(-)
>
> diff --git a/kernel/dma/Kconfig b/kernel/dma/Kconfig
> index d006668c0027d2..e0dae570a51530 100644
> --- a/kernel/dma/Kconfig
> +++ b/kernel/dma/Kconfig
> @@ -71,6 +71,7 @@ config SWIOTLB
> # in the pagetables
> #
> config DMA_NONCOHERENT_MMAP
> + default y if !MMU
> bool
>
> config DMA_REMAP
> diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
> index 0a4881e59aa7d6..9ec6a5c3fc578c 100644
> --- a/kernel/dma/direct.c
> +++ b/kernel/dma/direct.c
> @@ -459,7 +459,6 @@ int dma_direct_get_sgtable(struct device *dev, struct
> sg_table *sgt,
> return ret;
> }
>
> -#ifdef CONFIG_MMU
> bool dma_direct_can_mmap(struct device *dev)
> {
> return dev_is_dma_coherent(dev) ||
> @@ -485,19 +484,6 @@ int dma_direct_mmap(struct device *dev, struct
> vm_area_struct *vma,
> return remap_pfn_range(vma, vma->vm_start, pfn + vma->vm_pgoff,
> user_count << PAGE_SHIFT, vma->vm_page_prot);
> }
> -#else /* CONFIG_MMU */
> -bool dma_direct_can_mmap(struct device *dev)
> -{
> - return false;
> -}
> -
> -int dma_direct_mmap(struct device *dev, struct vm_area_struct *vma,
> - void *cpu_addr, dma_addr_t dma_addr, size_t size,
> - unsigned long attrs)
> -{
> - return -ENXIO;
> -}
> -#endif /* CONFIG_MMU */
>
> int dma_direct_supported(struct device *dev, u64 mask)
> {
>
LGTM. Should it go stable as well?
Cheers
Vladimir
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] iommu/amd: Fix event counter availability check
On 6/16/20 3:48 AM, Alexander Monakov wrote: Alexander On 6/1/20 4:01 PM, Alexander Monakov wrote: On Mon, 1 Jun 2020, Suravee Suthikulpanit wrote: Moving init_iommu_perf_ctr just after iommu_flush_all_caches resolves the issue. This is the earliest point in amd_iommu_init_pci where the call succeeds on my laptop. According to your description, it should just need to be anywhere after the pci_enable_device() is called for the IOMMU device, isn't it? So, on your system, what if we just move the init_iommu_perf_ctr() here: No, this doesn't work, as I already said in the paragraph you are responding to. See my last sentence in the quoted part. So the implication is init_device_table_dma together with subsequent cache flush is also setting up something that is necessary for counters to be writable. Alexander Instead of blindly moving the code around to a spot that would just work, I am trying to understand what might be required here. In this case, the init_device_table_dma()should not be needed. I suspect it's the IOMMU invalidate all command that's also needed here. I'm also checking with the HW and BIOS team. Meanwhile, could you please give the following change a try: Hello. Can you give any update please? Alexander Sorry for late reply. I have a reproducer and working with the HW team to understand the issue. I should be able to provide update with solution by the end of this week. Suravee ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 2/4] pci: set "untrusted" flag for truly external devices only
On Mon, Jun 15, 2020 at 06:17:40PM -0700, Rajat Jain wrote: > The "ExternalFacing" devices (root ports) are still internal devices > that sit on the internal system fabric and thus trusted. Currently they > were being marked untrusted - likely as an unintended border case. It was actually intentional :) At the time this was added we did not see benefits from doing this and even with this you actually are going to still miss things like a TBT chip that is soldered on the motherboard, I guess that can be though as an internal device as well. No objections to this patch, though. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 08/12] mm: Define pasid in mm
On Fri, Jun 12, 2020 at 05:41:29PM -0700, Fenghua Yu wrote:
> diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
> index 64ede5f150dc..5778db3aa42d 100644
> --- a/include/linux/mm_types.h
> +++ b/include/linux/mm_types.h
> @@ -538,6 +538,10 @@ struct mm_struct {
> atomic_long_t hugetlb_usage;
> #endif
> struct work_struct async_put_work;
> +
> +#ifdef CONFIG_PCI_PASID
Non-PCI devices can also use a PASID (e.g. Arm's SubstreamID). How about
CONFIG_IOMMU_SUPPORT?
Thanks,
Jean
> + unsigned int pasid;
> +#endif
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC][PATCH 5/5] firmware: QCOM_SCM: Allow qcom_scm driver to be loadable as a permenent module
Hi John,
+Will for the SMMU part.
On 2020-06-16 07:13, John Stultz wrote:
Allow the qcom_scm driver to be loadable as a
permenent module.
Cc: Andy Gross
Cc: Bjorn Andersson
Cc: Joerg Roedel
Cc: Thomas Gleixner
Cc: Jason Cooper
Cc: Marc Zyngier
Cc: Linus Walleij
Cc: Lina Iyer
Cc: Saravana Kannan
Cc: Todd Kjos
Cc: Greg Kroah-Hartman
Cc: linux-arm-...@vger.kernel.org
Cc: iommu@lists.linux-foundation.org
Cc: linux-g...@vger.kernel.org
Signed-off-by: John Stultz
---
drivers/firmware/Kconfig| 2 +-
drivers/firmware/Makefile | 3 ++-
drivers/firmware/qcom_scm.c | 4
drivers/iommu/Kconfig | 2 ++
4 files changed, 9 insertions(+), 2 deletions(-)
diff --git a/drivers/firmware/Kconfig b/drivers/firmware/Kconfig
index fbd785dd0513..9e533a462bf4 100644
--- a/drivers/firmware/Kconfig
+++ b/drivers/firmware/Kconfig
@@ -236,7 +236,7 @@ config INTEL_STRATIX10_RSU
Say Y here if you want Intel RSU support.
config QCOM_SCM
- bool
+ tristate "Qcom SCM driver"
depends on ARM || ARM64
select RESET_CONTROLLER
diff --git a/drivers/firmware/Makefile b/drivers/firmware/Makefile
index 99510be9f5ed..cf24d674216b 100644
--- a/drivers/firmware/Makefile
+++ b/drivers/firmware/Makefile
@@ -17,7 +17,8 @@ obj-$(CONFIG_ISCSI_IBFT) += iscsi_ibft.o
obj-$(CONFIG_FIRMWARE_MEMMAP) += memmap.o
obj-$(CONFIG_RASPBERRYPI_FIRMWARE) += raspberrypi.o
obj-$(CONFIG_FW_CFG_SYSFS) += qemu_fw_cfg.o
-obj-$(CONFIG_QCOM_SCM) += qcom_scm.o qcom_scm-smc.o qcom_scm-legacy.o
+obj-$(CONFIG_QCOM_SCM) += qcom-scm.o
+qcom-scm-objs += qcom_scm.o qcom_scm-smc.o qcom_scm-legacy.o
obj-$(CONFIG_TI_SCI_PROTOCOL) += ti_sci.o
obj-$(CONFIG_TRUSTED_FOUNDATIONS) += trusted_foundations.o
obj-$(CONFIG_TURRIS_MOX_RWTM) += turris-mox-rwtm.o
diff --git a/drivers/firmware/qcom_scm.c b/drivers/firmware/qcom_scm.c
index 0e7233a20f34..b5e88bf66975 100644
--- a/drivers/firmware/qcom_scm.c
+++ b/drivers/firmware/qcom_scm.c
@@ -1155,6 +1155,7 @@ static const struct of_device_id
qcom_scm_dt_match[] = {
{ .compatible = "qcom,scm" },
{}
};
+MODULE_DEVICE_TABLE(of, qcom_scm_dt_match);
static struct platform_driver qcom_scm_driver = {
.driver = {
@@ -1170,3 +1171,6 @@ static int __init qcom_scm_init(void)
return platform_driver_register(&qcom_scm_driver);
}
subsys_initcall(qcom_scm_init);
+
+MODULE_DESCRIPTION("Qualcomm Technologies, Inc. SCM driver");
+MODULE_LICENSE("GPL v2");
diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
index b510f67dfa49..714893535dd2 100644
--- a/drivers/iommu/Kconfig
+++ b/drivers/iommu/Kconfig
@@ -381,6 +381,7 @@ config SPAPR_TCE_IOMMU
config ARM_SMMU
tristate "ARM Ltd. System MMU (SMMU) Support"
depends on (ARM64 || ARM || (COMPILE_TEST && !GENERIC_ATOMIC64)) &&
MMU
+ depends on QCOM_SCM || !QCOM_SCM #if QCOM_SCM=m this can't be =y
This looks a bit ugly. Could you explain why we need this at the SMMU
level? I'd have expected the dependency to flow the other way around...
select IOMMU_API
select IOMMU_IO_PGTABLE_LPAE
select ARM_DMA_USE_IOMMU if ARM
@@ -500,6 +501,7 @@ config QCOM_IOMMU
# Note: iommu drivers cannot (yet?) be built as modules
bool "Qualcomm IOMMU Support"
depends on ARCH_QCOM || (COMPILE_TEST && !GENERIC_ATOMIC64)
+ depends on QCOM_SCM=y
select IOMMU_API
select IOMMU_IO_PGTABLE_LPAE
select ARM_DMA_USE_IOMMU
Thanks,
M.
--
Jazz is not dead. It just smells funny...
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 4/4] pci: export untrusted attribute in sysfs
On Mon, Jun 15, 2020 at 06:17:42PM -0700, Rajat Jain via iommu wrote: > This is needed to allow the userspace to determine when an untrusted > device has been added, and thus allowing it to bind the driver manually > to it, if it so wishes. This is being done as part of the approach > discussed at https://lkml.org/lkml/2020/6/9/1331 Please move the attribute to struct device instead of further entrenching it in PCIe. I'm starting to grow tired of saying this every other week while you guys are all moving into the entirely wrong direction. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
