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.
Thanks,
drew
