From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-io1-f43.google.com (mail-io1-f43.google.com [209.85.166.43]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id C342B10784 for ; Fri, 28 Jul 2023 08:48:45 +0000 (UTC) Received: by mail-io1-f43.google.com with SMTP id ca18e2360f4ac-783698a37beso81612439f.0 for ; Fri, 28 Jul 2023 01:48:45 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sifive.com; s=google; t=1690534124; x=1691138924; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=csmJdueaYe4TxgWwO4Wd3yP+V9n21yRw5VpjM04yUXk=; b=BM2vivVtM0Vi8txgqqrMMVb/gEDggT3Tp7zPzwqBmVjYZjyemJMBl2CnhFcmwoK3VQ 5stAE9+DdzguzRWN33KZEG+VBG8c7XDe5pfdv5xrw5IJYGmPCGgQ3i5oF9DJDcCmzGwK 6495KdKqI1WSIrcw87ndvyo3xlljoPnBbM/rPT14u/Z6dR8XWFQNUvHqbIu/pfbppDqW +GAyql6/CUacfDF7OnqUqwVrJxdk9Tjl6iSbHXyhbkDoczvCZaoniefnBZ6H5Vh6Akc4 VhPCu8cG8U2KsJmvs8g94sbhlmpRjxKqR8K17VLffXR/eTxQ2zxDnpHXrf4GgzKOqyBq MRgw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1690534124; x=1691138924; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=csmJdueaYe4TxgWwO4Wd3yP+V9n21yRw5VpjM04yUXk=; b=PuRm/fbuamHDAU2tO17teeA1x0Fe0nFQkytPt2oU0d9raU0lTq3icD6LWp4QrSX5fo kIqSQUwsyoXMO67dUjwHnZOGTN7LrEHncFh047HBEaiZIFNdC3CRt/PMgpnZn7th3VOY IM2v4ZD5R9Y3qz5HJYA9HL3ZVChmN35AkQFuSlCbNFH82HSYtq56M+y2KqfdQDw7c/de epOWNKbEftZlU4LL2QCLR7UPruOqIIVOdaHPs3KgHjGiEL9RzGeRiZA0JnUW22GiEFV7 3q1f4uoHLrjsG/enmZlj6TOf2c+V7miE6W4uB34rgwm4zZTytLgff+5ES4n5XHXXfGYq Dd5Q== X-Gm-Message-State: ABy/qLYFDfEfMV36gV+52NQixJlF5XdjTWmwckx+087yQYrdtCa113jw QftoTFkleREkXC40uw6VOCtwn2QmYYLxutR2UrzdEw== X-Google-Smtp-Source: APBJJlHXzbuDXRfZ01YQCScE+IuEs9Tr6or3RH84HjbXeEzCJBbzJyNhFuapVEmPS3Ihqc7dQPiElvgnG37NjgZYaT4= X-Received: by 2002:a5e:990d:0:b0:785:ff35:f340 with SMTP id t13-20020a5e990d000000b00785ff35f340mr2426775ioj.14.1690534124749; Fri, 28 Jul 2023 01:48:44 -0700 (PDT) Precedence: bulk X-Mailing-List: iommu@lists.linux.dev List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 References: <1fd79e5c53d9d6ed2264f60dd4261f293cc00472.1689792825.git.tjeznach@rivosinc.com> <5b8fd18e-8dfa-96bf-cdd4-4498b1d15ab9@ics.forth.gr> In-Reply-To: From: Zong Li Date: Fri, 28 Jul 2023 16:48:32 +0800 Message-ID: Subject: Re: [PATCH 06/11] RISC-V: drivers/iommu/riscv: Add command, fault, page-req queues To: Tomasz Jeznach Cc: Nick Kossifidis , Anup Patel , Albert Ou , linux@rivosinc.com, Will Deacon , Joerg Roedel , linux-kernel@vger.kernel.org, Sebastien Boeuf , iommu@lists.linux.dev, Palmer Dabbelt , Paul Walmsley , linux-riscv@lists.infradead.org, Robin Murphy Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Fri, Jul 28, 2023 at 1:19=E2=80=AFPM Tomasz Jeznach wrote: > > On Mon, Jul 24, 2023 at 11:47=E2=80=AFAM Zong Li wro= te: > > > > On Fri, Jul 21, 2023 at 2:00=E2=80=AFAM Tomasz Jeznach wrote: > > > > > > On Wed, Jul 19, 2023 at 8:12=E2=80=AFPM Nick Kossifidis wrote: > > > > > > > > Hello Tomasz, > > > > > > > > On 7/19/23 22:33, Tomasz Jeznach wrote: > > > > > Enables message or wire signal interrupts for PCIe and platforms = devices. > > > > > > > > > > > > > The description doesn't match the subject nor the patch content (we > > > > don't jus enable interrupts, we also init the queues). > > > > > > > > > + /* Parse Queue lengts */ > > > > > + ret =3D of_property_read_u32(pdev->dev.of_node, "cmdq_len",= &iommu->cmdq_len); > > > > > + if (!ret) > > > > > + dev_info(dev, "command queue length set to %i\n", i= ommu->cmdq_len); > > > > > + > > > > > + ret =3D of_property_read_u32(pdev->dev.of_node, "fltq_len",= &iommu->fltq_len); > > > > > + if (!ret) > > > > > + dev_info(dev, "fault/event queue length set to %i\n= ", iommu->fltq_len); > > > > > + > > > > > + ret =3D of_property_read_u32(pdev->dev.of_node, "priq_len",= &iommu->priq_len); > > > > > + if (!ret) > > > > > + dev_info(dev, "page request queue length set to %i\= n", iommu->priq_len); > > > > > + > > > > > dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(64)); > > > > > > > > > > > > > We need to add those to the device tree binding doc (or throw them = away, > > > > I thought it would be better to have them as part of the device > > > > desciption than a module parameter). > > > > > > > > > > We can add them as an optional fields to DT. > > > Alternatively, I've been looking into an option to auto-scale CQ/PQ > > > based on number of attached devices, but this gets trickier for > > > hot-pluggable systems. I've added module parameters as a bare-minimum= , > > > but still looking for better solutions. > > > > > > > > > > > > +static irqreturn_t riscv_iommu_priq_irq_check(int irq, void *dat= a); > > > > > +static irqreturn_t riscv_iommu_priq_process(int irq, void *data)= ; > > > > > + > > > > > > > > > + case RISCV_IOMMU_PAGE_REQUEST_QUEUE: > > > > > + q =3D &iommu->priq; > > > > > + q->len =3D sizeof(struct riscv_iommu_pq_record); > > > > > + count =3D iommu->priq_len; > > > > > + irq =3D iommu->irq_priq; > > > > > + irq_check =3D riscv_iommu_priq_irq_check; > > > > > + irq_process =3D riscv_iommu_priq_process; > > > > > + q->qbr =3D RISCV_IOMMU_REG_PQB; > > > > > + q->qcr =3D RISCV_IOMMU_REG_PQCSR; > > > > > + name =3D "priq"; > > > > > + break; > > > > > > > > > > > > It makes more sense to add the code for the page request queue in t= he > > > > patch that adds ATS/PRI support IMHO. This comment also applies to = its > > > > interrupt handlers below. > > > > > > > > > > ack. will do. > > > > > > > > > > > > +static inline void riscv_iommu_cmd_inval_set_addr(struct riscv_i= ommu_command *cmd, > > > > > + u64 addr) > > > > > +{ > > > > > + cmd->dword0 |=3D RISCV_IOMMU_CMD_IOTINVAL_AV; > > > > > + cmd->dword1 =3D addr; > > > > > +} > > > > > + > > > > > > > > This needs to be (addr >> 2) to match the spec, same as in the iofe= nce > > > > command. > > > > > > > > > > oops. Thanks! > > > > > > > I think it should be (addr >> 12) according to the spec. > > > > My reading of the spec '3.1.1. IOMMU Page-Table cache invalidation comman= ds' > is that it is a 4k page aligned address packed at dword1[61:10], so > effectively shifted by 2 bits. Thanks for your clarifying. Just an opinion, perhaps you can use 'FIELD_PREP()' on it as well, it might be clearer. > > regards, > - Tomasz > > > > > Regards, > > > > Nick > > > > > > > > > > regards, > > > - Tomasz > > > > > > _______________________________________________ > > > linux-riscv mailing list > > > linux-riscv@lists.infradead.org > > > http://lists.infradead.org/mailman/listinfo/linux-riscv