Re: [PATCH v5 11/13] hw/riscv/riscv-iommu: Add another irq for mrif notifications

[Andrew Jones <ajones@ventanamicro.com>]

On Wed, Jul 31, 2024 at 01:27:09PM GMT, Daniel Henrique Barboza wrote:
> Hi Jason,
> 
> 
> On 7/23/24 12:25 PM, Jason Chien wrote:
> > Hi Daniel,
> > 
> > On 2024/7/9 上午 01:34, Daniel Henrique Barboza wrote:
> > > From: Andrew Jones <ajones@ventanamicro.com>
> > > 
> > > And add mrif notification trace.
> > > 
> > > Signed-off-by: Andrew Jones <ajones@ventanamicro.com>
> > > Reviewed-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
> > > Reviewed-by: Frank Chang <frank.chang@sifive.com>
> > > ---
> > >   hw/riscv/riscv-iommu-pci.c | 2 +-
> > >   hw/riscv/riscv-iommu.c     | 1 +
> > >   hw/riscv/trace-events      | 1 +
> > >   3 files changed, 3 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/hw/riscv/riscv-iommu-pci.c b/hw/riscv/riscv-iommu-pci.c
> > > index 7b82ce0645..d7e5f20885 100644
> > > --- a/hw/riscv/riscv-iommu-pci.c
> > > +++ b/hw/riscv/riscv-iommu-pci.c
> > > @@ -81,7 +81,7 @@ static void riscv_iommu_pci_realize(PCIDevice *dev, Error **errp)
> > >       pci_register_bar(dev, 0, PCI_BASE_ADDRESS_SPACE_MEMORY |
> > >                        PCI_BASE_ADDRESS_MEM_TYPE_64, &s->bar0);
> > > -    int ret = msix_init(dev, RISCV_IOMMU_INTR_COUNT,
> > > +    int ret = msix_init(dev, RISCV_IOMMU_INTR_COUNT + 1,
> > The new interrupt is not marked as used with msix_vector_use().
> 
> I took at look at what this patch is actually doing and, at least in the MRIF setup
> I have, it's not doing much because we're not hitting the MRIF path inside the
> emulation. So we're not hitting the trace and this extra MSI isn't being used.
> 
> Drew is taking a look into it in the kernel side. Until we get a better idea on what's
> happening I'll remove this patch from the series. We can re-introduce it again later
> in this series or in the follow-up.

I recommend adding the trace to whatever patch introduces the MRIF path in
this series since we'll want the trace for testing regardless. If we need
another fix to this series for MRIFs then I'll post that separately on
top.

Thanks,
drew