From: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
To: Andrew Jones <ajones@ventanamicro.com>,
Frank Chang <frank.chang@sifive.com>
Cc: qemu-devel@nongnu.org, qemu-riscv@nongnu.org,
alistair.francis@wdc.com, bmeng@tinylab.org, liwei1518@gmail.com,
zhiwei_liu@linux.alibaba.com, palmer@rivosinc.com,
tjeznach@rivosinc.com
Subject: Re: [PATCH v2 09/15] hw/riscv/riscv-iommu: add s-stage and g-stage support
Date: Thu, 16 May 2024 16:41:33 -0300 [thread overview]
Message-ID: <f8860f42-cf5d-4afa-aa38-964434238844@ventanamicro.com> (raw)
In-Reply-To: <20240510-dbedfaea5903daa73f461e2b@orel>
On 5/10/24 08:14, Andrew Jones wrote:
> On Fri, May 10, 2024 at 06:36:51PM GMT, Frank Chang wrote:
> ...
>>> static int riscv_iommu_spa_fetch(RISCVIOMMUState *s, RISCVIOMMUContext *ctx,
>>> - IOMMUTLBEntry *iotlb)
>>> + IOMMUTLBEntry *iotlb, bool gpa)
>>> {
>>> + dma_addr_t addr, base;
>>> + uint64_t satp, gatp, pte;
>>> + bool en_s, en_g;
>>> + struct {
>>> + unsigned char step;
>>> + unsigned char levels;
>>> + unsigned char ptidxbits;
>>> + unsigned char ptesize;
>>> + } sc[2];
>>> + /* Translation stage phase */
>>> + enum {
>>> + S_STAGE = 0,
>>> + G_STAGE = 1,
>>> + } pass;
>>> +
>>> + satp = get_field(ctx->satp, RISCV_IOMMU_ATP_MODE_FIELD);
>>> + gatp = get_field(ctx->gatp, RISCV_IOMMU_ATP_MODE_FIELD);
>>> +
>>> + en_s = satp != RISCV_IOMMU_DC_FSC_MODE_BARE && !gpa;
>>> + en_g = gatp != RISCV_IOMMU_DC_IOHGATP_MODE_BARE;
>>> +
>>> /* Early check for MSI address match when IOVA == GPA */
>>> - if (iotlb->perm & IOMMU_WO &&
>>> + if (!en_s && (iotlb->perm & IOMMU_WO) &&
>>
>> I'm wondering do we need to check "en_s" for MSI writes?
>>
>> IOMMU spec Section 2.3.3. Process to translate addresses of MSIs says:
>> "Determine if the address A is an access to a virtual interrupt file
>> as specified in Section 2.1.3.6."
>>
>> and Section 2.1.3.6 says:
>>
>> "An incoming memory access made by a device is recognized as
>> an access to a virtual interrupt file if the destination guest physical page
>> matches the supplied address pattern in all bit positions that are zeros
>> in the supplied address mask. In detail, a memory access to
>> guest physical address A is recognized as an access to a virtual
>> interrupt file’s
>> memory-mapped page if:
>> (A >> 12) & ~msi_addr_mask = (msi_addr_pattern & ~msi_addr_mask)"
>>
>> Is checking the address pattern sufficient enough to determine
>> the address is an MSI to a virtual interrupt file?
>>
>
> I think so. In fact, I've removed that en_s check on our internal build in
> order to get things working for my irqbypass work, as we can do device
> assignment with VFIO with only S-stage enabled.
The following code will be fixed up here:
static int riscv_iommu_spa_fetch(RISCVIOMMUState *s, RISCVIOMMUContext *ctx,
- IOMMUTLBEntry *iotlb, bool gpa)
+ IOMMUTLBEntry *iotlb)
{
dma_addr_t addr, base;
uint64_t satp, gatp, pte;
@@ -238,11 +237,11 @@ static int riscv_iommu_spa_fetch(RISCVIOMMUState *s, RISCVIOMMUContext *ctx,
satp = get_field(ctx->satp, RISCV_IOMMU_ATP_MODE_FIELD);
gatp = get_field(ctx->gatp, RISCV_IOMMU_ATP_MODE_FIELD);
- en_s = satp != RISCV_IOMMU_DC_FSC_MODE_BARE && !gpa;
+ en_s = satp != RISCV_IOMMU_DC_FSC_MODE_BARE;
en_g = gatp != RISCV_IOMMU_DC_IOHGATP_MODE_BARE;
/* Early check for MSI address match when IOVA == GPA */
- if (!en_s && (iotlb->perm & IOMMU_WO) &&
+ if ((iotlb->perm & IOMMU_WO) &&
riscv_iommu_msi_check(s, ctx, iotlb->iova)) {
iotlb->target_as = &s->trap_as;
iotlb->translated_addr = iotlb->iova;
@@ -1203,7 +1202,7 @@ static int riscv_iommu_translate(RISCVIOMMUState *s, RISCVIOMMUContext *ctx,
}
/* Translate using device directory / page table information. */
- fault = riscv_iommu_spa_fetch(s, ctx, iotlb, false);
+ fault = riscv_iommu_spa_fetch(s, ctx, iotlb);
if (!fault && iotlb->target_as == &s->trap_as) {
/* Do not cache trapped MSI translations */
'gpa' is eliminated since it was only being used as 'false' by the only
caller of riscv_iommu_spa_fetch(). The boolean was used only to calculate
en_s as "&& !gpa", so it's always 'true' and had no impact in en_s. My
understand here is that 'gpa' was a prototype of the first implementation
that got left behind and ended up not being used.
As for the MSI check, we won't skip translation if satp is bare (!en_s) because
we might be using just stage2 for a guest, thus en_s is removed from the
conditional. As Frank said, this change also complies with the spec since we don't
need to check satp to determine if the address is an MSI to a virtual interrupt
file.
And, last but not the least, this change doesn't break my KVM VFIO passthrough
test case :) I'll document more about the test case I'm using in the v3 cover
letter.
Thanks,
Daniel
>
> Thanks,
> drew
next prev parent reply other threads:[~2024-05-16 19:42 UTC|newest]
Thread overview: 55+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-03-07 16:03 [PATCH v2 00/15] riscv: QEMU RISC-V IOMMU Support Daniel Henrique Barboza
2024-03-07 16:03 ` [PATCH v2 01/15] exec/memtxattr: add process identifier to the transaction attributes Daniel Henrique Barboza
2024-04-23 16:33 ` Frank Chang
2024-03-07 16:03 ` [PATCH v2 02/15] hw/riscv: add riscv-iommu-bits.h Daniel Henrique Barboza
2024-05-10 11:01 ` Frank Chang
2024-05-15 10:02 ` Eric Cheng
2024-05-15 14:28 ` Daniel Henrique Barboza
2024-03-07 16:03 ` [PATCH v2 03/15] hw/riscv: add RISC-V IOMMU base emulation Daniel Henrique Barboza
2024-05-01 11:57 ` Jason Chien
2024-05-14 20:06 ` Daniel Henrique Barboza
2024-05-02 11:37 ` Frank Chang
2024-05-08 11:15 ` Daniel Henrique Barboza
2024-05-10 10:58 ` Frank Chang
2024-05-13 12:41 ` Daniel Henrique Barboza
2024-05-13 12:37 ` Daniel Henrique Barboza
2024-05-16 7:13 ` Frank Chang
2024-05-20 16:17 ` Daniel Henrique Barboza
2024-05-21 10:52 ` Frank Chang
2024-05-21 12:28 ` Daniel Henrique Barboza
2024-03-07 16:03 ` [PATCH v2 04/15] hw/riscv: add riscv-iommu-pci device Daniel Henrique Barboza
2024-04-29 7:21 ` Frank Chang
2024-05-02 9:37 ` Daniel Henrique Barboza
2024-03-07 16:03 ` [PATCH v2 05/15] hw/riscv: add riscv-iommu-sys platform device Daniel Henrique Barboza
2024-04-30 1:35 ` Frank Chang
2024-03-07 16:03 ` [PATCH v2 06/15] hw/riscv/virt.c: support for RISC-V IOMMU PCIDevice hotplug Daniel Henrique Barboza
2024-04-30 2:17 ` Frank Chang
2024-05-15 6:25 ` Eric Cheng
2024-05-15 7:16 ` Andrew Jones
2024-03-07 16:03 ` [PATCH v2 07/15] test/qtest: add riscv-iommu-pci tests Daniel Henrique Barboza
2024-04-30 3:33 ` Frank Chang
2024-03-07 16:03 ` [PATCH v2 08/15] hw/riscv/riscv-iommu: add Address Translation Cache (IOATC) Daniel Henrique Barboza
2024-05-08 7:26 ` Frank Chang
2024-05-16 21:45 ` Daniel Henrique Barboza
2024-03-07 16:03 ` [PATCH v2 09/15] hw/riscv/riscv-iommu: add s-stage and g-stage support Daniel Henrique Barboza
2024-05-10 10:36 ` Frank Chang
2024-05-10 11:14 ` Andrew Jones
2024-05-16 19:41 ` Daniel Henrique Barboza [this message]
2024-03-07 16:03 ` [PATCH v2 10/15] hw/riscv/riscv-iommu: add ATS support Daniel Henrique Barboza
2024-05-08 2:57 ` Frank Chang
2024-05-17 9:29 ` Daniel Henrique Barboza
2024-03-07 16:03 ` [PATCH v2 11/15] hw/riscv/riscv-iommu: add DBG support Daniel Henrique Barboza
2024-05-06 4:09 ` Frank Chang
2024-05-06 13:05 ` Daniel Henrique Barboza
2024-05-10 10:59 ` Frank Chang
2024-03-07 16:03 ` [PATCH v2 12/15] hw/riscv/riscv-iommu: Add another irq for mrif notifications Daniel Henrique Barboza
2024-05-06 6:12 ` Frank Chang
2024-03-07 16:03 ` [PATCH v2 13/15] qtest/riscv-iommu-test: add init queues test Daniel Henrique Barboza
2024-05-07 8:01 ` Frank Chang
2024-03-07 16:03 ` [PATCH v2 14/15] hw/misc: EDU: added PASID support Daniel Henrique Barboza
2024-05-07 9:06 ` Frank Chang
2024-03-07 16:03 ` [PATCH v2 15/15] hw/misc: EDU: add ATS/PRI capability Daniel Henrique Barboza
2024-05-07 15:32 ` Frank Chang
2024-05-16 13:59 ` Daniel Henrique Barboza
2024-05-10 11:14 ` [PATCH v2 00/15] riscv: QEMU RISC-V IOMMU Support Frank Chang
2024-05-20 16:26 ` Daniel Henrique Barboza
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=f8860f42-cf5d-4afa-aa38-964434238844@ventanamicro.com \
--to=dbarboza@ventanamicro.com \
--cc=ajones@ventanamicro.com \
--cc=alistair.francis@wdc.com \
--cc=bmeng@tinylab.org \
--cc=frank.chang@sifive.com \
--cc=liwei1518@gmail.com \
--cc=palmer@rivosinc.com \
--cc=qemu-devel@nongnu.org \
--cc=qemu-riscv@nongnu.org \
--cc=tjeznach@rivosinc.com \
--cc=zhiwei_liu@linux.alibaba.com \
/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 read-only IMAP folder(s) and NNTP newsgroup(s).