From: "Li, Chunming" <Chunming.Li@verisilicon.com>
To: "eric.auger@redhat.com" <eric.auger@redhat.com>,
chunming <chunming_li1234@163.com>,
"peter.maydell@linaro.org" <peter.maydell@linaro.org>
Cc: "Liu, Renwei" <Renwei.Liu@verisilicon.com>,
"qemu-arm@nongnu.org" <qemu-arm@nongnu.org>,
"Wen, Jianxian" <Jianxian.Wen@verisilicon.com>,
"qemu-devel@nongnu.org" <qemu-devel@nongnu.org>
Subject: RE: [PATCH v5 2/4] hw/arm/smmuv3: Update implementation of CFGI commands based on device SID
Date: Wed, 1 Sep 2021 06:51:19 +0000 [thread overview]
Message-ID: <49C79B700B5D8F45B8EF0861B4EF3B3B0114302B8C@SHASXM03.verisilicon.com> (raw)
In-Reply-To: <7089ce3e-2b15-7cf3-86d9-231c69794138@redhat.com>
> -----Original Message-----
> From: Eric Auger [mailto:eric.auger@redhat.com]
> Sent: Tuesday, August 31, 2021 10:02 PM
> To: chunming; peter.maydell@linaro.org
> Cc: qemu-arm@nongnu.org; qemu-devel@nongnu.org; Wen, Jianxian; Liu,
> Renwei; Li, Chunming
> Subject: Re: [PATCH v5 2/4] hw/arm/smmuv3: Update implementation of
> CFGI commands based on device SID
>
> Hi Chunming
>
> On 8/25/21 10:08 AM, chunming wrote:
> > From: chunming <chunming.li@verisilicon.com>
> >
> > Replace "smmuv3_flush_config" with "g_hash_table_foreach_remove".
> this replacement may have a potential negative impact on the
> performance
> for PCIe support, which is the main use case: a unique
> g_hash_table_remove() is replaced by an iteration over all the config
> hash keys.
>
> I wonder if you couldn't just adapt smmu_iommu_mr() and it case this
> latter returns NULL for the current PCIe search, look up in the
> platform
> device list:
>
> peri_sdev_list?
I think there are 2 scenes:
1. PCIe devices share same SID with peripheral devices.
2. Multi peripheral devices share same SID.
If we search PCIe 1st then search peri_sdev_list, there are 2 problems:
1. The code is complex.
2. We may need to search peri_sdev_list multi times. It may has performance impact.
The CFGI commands are only called when the SMMU device is removed.
So we think there is no big performance impact.
>
> Thanks
>
> Eric
>
>
>
> > "smmu_iommu_mr" function can't get MR according to SID for non
> PCI/PCIe devices.
> >
> > Signed-off-by: chunming <chunming.li@verisilicon.com>
> > ---
> > hw/arm/smmuv3.c | 35 ++++++++++------------------------
> -
> > include/hw/arm/smmu-common.h | 5 ++++-
> > 2 files changed, 14 insertions(+), 26 deletions(-)
> >
> > diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
> > index 11d7fe8423..9f3f13fb8e 100644
> > --- a/hw/arm/smmuv3.c
> > +++ b/hw/arm/smmuv3.c
> > @@ -613,14 +613,6 @@ static SMMUTransCfg
> *smmuv3_get_config(SMMUDevice *sdev, SMMUEventInfo *event)
> > return cfg;
> > }
> >
> > -static void smmuv3_flush_config(SMMUDevice *sdev)
> > -{
> > - SMMUv3State *s = sdev->smmu;
> > - SMMUState *bc = &s->smmu_state;
> > -
> > - trace_smmuv3_config_cache_inv(smmu_get_sid(sdev));
> > - g_hash_table_remove(bc->configs, sdev);
> > -}
> >
> > static IOMMUTLBEntry smmuv3_translate(IOMMUMemoryRegion *mr, hwaddr
> addr,
> > IOMMUAccessFlags flag, int
> iommu_idx)
> > @@ -964,22 +956,18 @@ static int smmuv3_cmdq_consume(SMMUv3State *s)
> > case SMMU_CMD_CFGI_STE:
> > {
> > uint32_t sid = CMD_SID(&cmd);
> > - IOMMUMemoryRegion *mr = smmu_iommu_mr(bs, sid);
> > - SMMUDevice *sdev;
> > + SMMUSIDRange sid_range;
> >
> > if (CMD_SSEC(&cmd)) {
> > cmd_error = SMMU_CERROR_ILL;
> > break;
> > }
> >
> > - if (!mr) {
> > - break;
> > - }
> > -
> > + sid_range.start = sid;
> > + sid_range.end = sid;
> > trace_smmuv3_cmdq_cfgi_ste(sid);
> > - sdev = container_of(mr, SMMUDevice, iommu);
> > - smmuv3_flush_config(sdev);
> > -
> > + g_hash_table_foreach_remove(bs->configs,
> smmuv3_invalidate_ste,
> > + &sid_range);
> > break;
> > }
> > case SMMU_CMD_CFGI_STE_RANGE: /* same as SMMU_CMD_CFGI_ALL
> */
> > @@ -1006,21 +994,18 @@ static int smmuv3_cmdq_consume(SMMUv3State *s)
> > case SMMU_CMD_CFGI_CD_ALL:
> > {
> > uint32_t sid = CMD_SID(&cmd);
> > - IOMMUMemoryRegion *mr = smmu_iommu_mr(bs, sid);
> > - SMMUDevice *sdev;
> > + SMMUSIDRange sid_range;
> >
> > if (CMD_SSEC(&cmd)) {
> > cmd_error = SMMU_CERROR_ILL;
> > break;
> > }
> >
> > - if (!mr) {
> > - break;
> > - }
> > -
> > + sid_range.start = sid;
> > + sid_range.end = sid;
> > trace_smmuv3_cmdq_cfgi_cd(sid);
> > - sdev = container_of(mr, SMMUDevice, iommu);
> > - smmuv3_flush_config(sdev);
> > + g_hash_table_foreach_remove(bs->configs,
> smmuv3_invalidate_ste,
> > + &sid_range);
> > break;
> > }
> > case SMMU_CMD_TLBI_NH_ASID:
> > diff --git a/include/hw/arm/smmu-common.h b/include/hw/arm/smmu-
> common.h
> > index 95cd12a4b5..d016455d80 100644
> > --- a/include/hw/arm/smmu-common.h
> > +++ b/include/hw/arm/smmu-common.h
> > @@ -159,7 +159,10 @@ int smmu_ptw(SMMUTransCfg *cfg, dma_addr_t iova,
> IOMMUAccessFlags perm,
> > */
> > SMMUTransTableInfo *select_tt(SMMUTransCfg *cfg, dma_addr_t iova);
> >
> > -/* Return the iommu mr associated to @sid, or NULL if none */
> > +/**
> > + * Return the iommu mr associated to @sid, or NULL if none
> > + * Only for PCI device, check smmu_find_peri_sdev for non PCI/PCIe
> device
> > + */
> > IOMMUMemoryRegion *smmu_iommu_mr(SMMUState *s, uint32_t sid);
> >
> > #define SMMU_IOTLB_MAX_SIZE 256
next prev parent reply other threads:[~2021-09-01 6:54 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-08-25 8:08 [PATCH v5 0/4] hw/arm/smmuv3: Support non PCI/PCIe devices chunming
2021-08-25 8:08 ` [PATCH v5 1/4] hw/arm/smmuv3: Support non PCI/PCIe device connect with SMMU v3 chunming
2021-08-31 14:01 ` Eric Auger
2021-09-01 6:51 ` Li, Chunming
2021-08-25 8:08 ` [PATCH v5 2/4] hw/arm/smmuv3: Update implementation of CFGI commands based on device SID chunming
2021-08-31 14:01 ` Eric Auger
2021-09-01 6:51 ` Li, Chunming [this message]
2021-09-01 12:04 ` Eric Auger
2021-09-02 1:59 ` Li, Chunming
2021-08-25 8:08 ` [PATCH v5 3/4] hw/arm/virt: Update SMMU v3 creation to support non PCI/PCIe device connection chunming
2021-08-31 14:13 ` Eric Auger
2021-09-01 6:51 ` Li, Chunming
2021-08-25 8:08 ` [PATCH v5 4/4] hw/arm/virt: Add PL330 DMA controller and connect with SMMU v3 chunming
2021-08-31 14:36 ` Eric Auger
2021-09-01 6:53 ` Li, Chunming
2021-09-01 10:22 ` Eric Auger
2021-09-02 6:46 ` Li, Chunming
2021-09-02 7:45 ` Eric Auger
2021-09-02 8:22 ` Li, Chunming
2021-09-07 11:00 ` Peter Maydell
2021-09-07 17:02 ` Leif Lindholm
2021-09-07 9:01 ` Li, Chunming
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=49C79B700B5D8F45B8EF0861B4EF3B3B0114302B8C@SHASXM03.verisilicon.com \
--to=chunming.li@verisilicon.com \
--cc=Jianxian.Wen@verisilicon.com \
--cc=Renwei.Liu@verisilicon.com \
--cc=chunming_li1234@163.com \
--cc=eric.auger@redhat.com \
--cc=peter.maydell@linaro.org \
--cc=qemu-arm@nongnu.org \
--cc=qemu-devel@nongnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).