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

Daniel Henrique Barboza posted 13 patches 4 months, 2 weeks ago
Maintainers: "Michael S. Tsirkin" <mst@redhat.com>, Marcel Apfelbaum <marcel.apfelbaum@gmail.com>, Paolo Bonzini <pbonzini@redhat.com>, Palmer Dabbelt <palmer@dabbelt.com>, Alistair Francis <alistair.francis@wdc.com>, Bin Meng <bmeng.cn@gmail.com>, Weiwei Li <liwei1518@gmail.com>, Daniel Henrique Barboza <dbarboza@ventanamicro.com>, Liu Zhiwei <zhiwei_liu@linux.alibaba.com>, "Marc-André Lureau" <marcandre.lureau@redhat.com>, "Daniel P. Berrangé" <berrange@redhat.com>, Thomas Huth <thuth@redhat.com>, "Philippe Mathieu-Daudé" <philmd@linaro.org>, Laurent Vivier <lvivier@redhat.com>
There is a newer version of this series
[PATCH v5 11/13] hw/riscv/riscv-iommu: Add another irq for mrif notifications
Posted by Daniel Henrique Barboza 4 months, 2 weeks ago
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,
                         &s->bar0, 0, RISCV_IOMMU_REG_MSI_CONFIG,
                         &s->bar0, 0, RISCV_IOMMU_REG_MSI_CONFIG + 256, 0, &err);
 
diff --git a/hw/riscv/riscv-iommu.c b/hw/riscv/riscv-iommu.c
index 2985a49e53..c9ac3d348b 100644
--- a/hw/riscv/riscv-iommu.c
+++ b/hw/riscv/riscv-iommu.c
@@ -621,6 +621,7 @@ static MemTxResult riscv_iommu_msi_write(RISCVIOMMUState *s,
         cause = RISCV_IOMMU_FQ_CAUSE_MSI_WR_FAULT;
         goto err;
     }
+    trace_riscv_iommu_mrif_notification(s->parent_obj.id, n190, addr);
 
     return MEMTX_OK;
 
diff --git a/hw/riscv/trace-events b/hw/riscv/trace-events
index 4b486b6420..d69719a27a 100644
--- a/hw/riscv/trace-events
+++ b/hw/riscv/trace-events
@@ -6,6 +6,7 @@ riscv_iommu_flt(const char *id, unsigned b, unsigned d, unsigned f, uint64_t rea
 riscv_iommu_pri(const char *id, unsigned b, unsigned d, unsigned f, uint64_t iova) "%s: page request %04x:%02x.%u iova: 0x%"PRIx64
 riscv_iommu_dma(const char *id, unsigned b, unsigned d, unsigned f, unsigned pasid, const char *dir, uint64_t iova, uint64_t phys) "%s: translate %04x:%02x.%u #%u %s 0x%"PRIx64" -> 0x%"PRIx64
 riscv_iommu_msi(const char *id, unsigned b, unsigned d, unsigned f, uint64_t iova, uint64_t phys) "%s: translate %04x:%02x.%u MSI 0x%"PRIx64" -> 0x%"PRIx64
+riscv_iommu_mrif_notification(const char *id, uint32_t nid, uint64_t phys) "%s: sent MRIF notification 0x%x to 0x%"PRIx64
 riscv_iommu_cmd(const char *id, uint64_t l, uint64_t u) "%s: command 0x%"PRIx64" 0x%"PRIx64
 riscv_iommu_notifier_add(const char *id) "%s: dev-iotlb notifier added"
 riscv_iommu_notifier_del(const char *id) "%s: dev-iotlb notifier removed"
-- 
2.45.2
Re: [PATCH v5 11/13] hw/riscv/riscv-iommu: Add another irq for mrif notifications
Posted by Jason Chien 4 months ago
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().
>                           &s->bar0, 0, RISCV_IOMMU_REG_MSI_CONFIG,
>                           &s->bar0, 0, RISCV_IOMMU_REG_MSI_CONFIG + 256, 0, &err);
>   
> diff --git a/hw/riscv/riscv-iommu.c b/hw/riscv/riscv-iommu.c
> index 2985a49e53..c9ac3d348b 100644
> --- a/hw/riscv/riscv-iommu.c
> +++ b/hw/riscv/riscv-iommu.c
> @@ -621,6 +621,7 @@ static MemTxResult riscv_iommu_msi_write(RISCVIOMMUState *s,
>           cause = RISCV_IOMMU_FQ_CAUSE_MSI_WR_FAULT;
>           goto err;
>       }
> +    trace_riscv_iommu_mrif_notification(s->parent_obj.id, n190, addr);
>   
>       return MEMTX_OK;
>   
> diff --git a/hw/riscv/trace-events b/hw/riscv/trace-events
> index 4b486b6420..d69719a27a 100644
> --- a/hw/riscv/trace-events
> +++ b/hw/riscv/trace-events
> @@ -6,6 +6,7 @@ riscv_iommu_flt(const char *id, unsigned b, unsigned d, unsigned f, uint64_t rea
>   riscv_iommu_pri(const char *id, unsigned b, unsigned d, unsigned f, uint64_t iova) "%s: page request %04x:%02x.%u iova: 0x%"PRIx64
>   riscv_iommu_dma(const char *id, unsigned b, unsigned d, unsigned f, unsigned pasid, const char *dir, uint64_t iova, uint64_t phys) "%s: translate %04x:%02x.%u #%u %s 0x%"PRIx64" -> 0x%"PRIx64
>   riscv_iommu_msi(const char *id, unsigned b, unsigned d, unsigned f, uint64_t iova, uint64_t phys) "%s: translate %04x:%02x.%u MSI 0x%"PRIx64" -> 0x%"PRIx64
> +riscv_iommu_mrif_notification(const char *id, uint32_t nid, uint64_t phys) "%s: sent MRIF notification 0x%x to 0x%"PRIx64
>   riscv_iommu_cmd(const char *id, uint64_t l, uint64_t u) "%s: command 0x%"PRIx64" 0x%"PRIx64
>   riscv_iommu_notifier_add(const char *id) "%s: dev-iotlb notifier added"
>   riscv_iommu_notifier_del(const char *id) "%s: dev-iotlb notifier removed"

Re: [PATCH v5 11/13] hw/riscv/riscv-iommu: Add another irq for mrif notifications
Posted by Daniel Henrique Barboza 3 months, 3 weeks ago
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.


Thanks,

Daniel

>>                           &s->bar0, 0, RISCV_IOMMU_REG_MSI_CONFIG,
>>                           &s->bar0, 0, RISCV_IOMMU_REG_MSI_CONFIG + 256, 0, &err);
>> diff --git a/hw/riscv/riscv-iommu.c b/hw/riscv/riscv-iommu.c
>> index 2985a49e53..c9ac3d348b 100644
>> --- a/hw/riscv/riscv-iommu.c
>> +++ b/hw/riscv/riscv-iommu.c
>> @@ -621,6 +621,7 @@ static MemTxResult riscv_iommu_msi_write(RISCVIOMMUState *s,
>>           cause = RISCV_IOMMU_FQ_CAUSE_MSI_WR_FAULT;
>>           goto err;
>>       }
>> +    trace_riscv_iommu_mrif_notification(s->parent_obj.id, n190, addr);
>>       return MEMTX_OK;
>> diff --git a/hw/riscv/trace-events b/hw/riscv/trace-events
>> index 4b486b6420..d69719a27a 100644
>> --- a/hw/riscv/trace-events
>> +++ b/hw/riscv/trace-events
>> @@ -6,6 +6,7 @@ riscv_iommu_flt(const char *id, unsigned b, unsigned d, unsigned f, uint64_t rea
>>   riscv_iommu_pri(const char *id, unsigned b, unsigned d, unsigned f, uint64_t iova) "%s: page request %04x:%02x.%u iova: 0x%"PRIx64
>>   riscv_iommu_dma(const char *id, unsigned b, unsigned d, unsigned f, unsigned pasid, const char *dir, uint64_t iova, uint64_t phys) "%s: translate %04x:%02x.%u #%u %s 0x%"PRIx64" -> 0x%"PRIx64
>>   riscv_iommu_msi(const char *id, unsigned b, unsigned d, unsigned f, uint64_t iova, uint64_t phys) "%s: translate %04x:%02x.%u MSI 0x%"PRIx64" -> 0x%"PRIx64
>> +riscv_iommu_mrif_notification(const char *id, uint32_t nid, uint64_t phys) "%s: sent MRIF notification 0x%x to 0x%"PRIx64
>>   riscv_iommu_cmd(const char *id, uint64_t l, uint64_t u) "%s: command 0x%"PRIx64" 0x%"PRIx64
>>   riscv_iommu_notifier_add(const char *id) "%s: dev-iotlb notifier added"
>>   riscv_iommu_notifier_del(const char *id) "%s: dev-iotlb notifier removed"

Re: [PATCH v5 11/13] hw/riscv/riscv-iommu: Add another irq for mrif notifications
Posted by Andrew Jones 3 months, 3 weeks ago
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

Re: [PATCH v5 11/13] hw/riscv/riscv-iommu: Add another irq for mrif notifications
Posted by Daniel Henrique Barboza 3 months, 3 weeks ago

On 7/31/24 1:50 PM, Andrew Jones wrote:
> 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.

I'll add the trace in patch 3 (or 9).


Daniel


> 
> Thanks,
> drew