[PATCH 0/3] Enhancing Device Identification in RISC-V IOMMU Using Memory Attributes

[PATCH 0/3] Enhancing Device Identification in RISC-V IOMMU Using Memory Attributes

[Jason Chien]

This patch series enhances how device IDs are handled in RISC-V IOMMU by
leveraging memory attributes.

The BDF (Bus-Device-Function) is now included in memory attributes for
DMA operations, ensuring accurate device identification.

Since PCIe bus numbers can change after re-enumeration, relying on static
device IDs in RISCVIOMMUSpace may lead to incorrect Device Directory Table
walk. The IOMMU now dynamically retrieves latest device IDs from memory
attributes.

The bus property, previously used to set non-root endpoint bus numbers,
is removed. As PCIe bus numbers are assigned at runtime and vary across
endpoints, exposing a single property to pre-set them is unnecessary and
incorrect. With device IDs now retrieved dynamically, this property is no
longer required.

Jason Chien (3):
  include/hw/pci: Attach BDF to Memory Attributes
  hw/riscv/riscv-iommu: Obtain Device IDs from Memory Attributes
  hw/riscv/riscv_iommu: Remove the "bus" property

 hw/riscv/riscv-iommu.c      | 15 +++++++--------
 hw/riscv/riscv-iommu.h      |  1 -
 include/hw/pci/pci_device.h | 10 ++++++++--
 3 files changed, 15 insertions(+), 11 deletions(-)

-- 
2.43.2

[PATCH 1/3] include/hw/pci: Attach BDF to Memory Attributes

[Jason Chien]

This commit adds the BDF to the memory attributes for DMA operations.

Signed-off-by: Jason Chien <jason.chien@sifive.com>
---
 include/hw/pci/pci_device.h | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/include/hw/pci/pci_device.h b/include/hw/pci/pci_device.h
index add208edfa..968f1ba3e9 100644
--- a/include/hw/pci/pci_device.h
+++ b/include/hw/pci/pci_device.h
@@ -244,6 +244,8 @@ static inline MemTxResult pci_dma_rw(PCIDevice *dev, dma_addr_t addr,
                                      void *buf, dma_addr_t len,
                                      DMADirection dir, MemTxAttrs attrs)
 {
+    attrs.unspecified = 0;
+    attrs.requester_id = pci_requester_id(dev);
     return dma_memory_rw(pci_get_address_space(dev), addr, buf, len,
                          dir, attrs);
 }
@@ -292,6 +294,8 @@ static inline MemTxResult pci_dma_write(PCIDevice *dev, dma_addr_t addr,
                                                uint##_bits##_t *val, \
                                                MemTxAttrs attrs) \
     { \
+        attrs.unspecified = 0; \
+        attrs.requester_id = pci_requester_id(dev); \
         return ld##_l##_dma(pci_get_address_space(dev), addr, val, attrs); \
     } \
     static inline MemTxResult st##_s##_pci_dma(PCIDevice *dev, \
@@ -299,6 +303,8 @@ static inline MemTxResult pci_dma_write(PCIDevice *dev, dma_addr_t addr,
                                                uint##_bits##_t val, \
                                                MemTxAttrs attrs) \
     { \
+        attrs.unspecified = 0; \
+        attrs.requester_id = pci_requester_id(dev); \
         return st##_s##_dma(pci_get_address_space(dev), addr, val, attrs); \
     }
 
@@ -327,8 +333,8 @@ PCI_DMA_DEFINE_LDST(q_be, q_be, 64);
 static inline void *pci_dma_map(PCIDevice *dev, dma_addr_t addr,
                                 dma_addr_t *plen, DMADirection dir)
 {
-    return dma_memory_map(pci_get_address_space(dev), addr, plen, dir,
-                          MEMTXATTRS_UNSPECIFIED);
+    MemTxAttrs attrs = {.requester_id = pci_requester_id(dev)};
+    return dma_memory_map(pci_get_address_space(dev), addr, plen, dir, attrs);
 }
 
 static inline void pci_dma_unmap(PCIDevice *dev, void *buffer, dma_addr_t len,
-- 
2.43.2

Re: [PATCH 1/3] include/hw/pci: Attach BDF to Memory Attributes

[Daniel Henrique Barboza]

On 3/2/25 6:12 AM, Jason Chien wrote:
> This commit adds the BDF to the memory attributes for DMA operations.
> 
> Signed-off-by: Jason Chien <jason.chien@sifive.com>
> ---

This looks sensible but I'll feel more comfortable if Michael/Marcel also
takes a look. Thanks,


Daniel

>   include/hw/pci/pci_device.h | 10 ++++++++--
>   1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/include/hw/pci/pci_device.h b/include/hw/pci/pci_device.h
> index add208edfa..968f1ba3e9 100644
> --- a/include/hw/pci/pci_device.h
> +++ b/include/hw/pci/pci_device.h
> @@ -244,6 +244,8 @@ static inline MemTxResult pci_dma_rw(PCIDevice *dev, dma_addr_t addr,
>                                        void *buf, dma_addr_t len,
>                                        DMADirection dir, MemTxAttrs attrs)
>   {
> +    attrs.unspecified = 0;
> +    attrs.requester_id = pci_requester_id(dev);
>       return dma_memory_rw(pci_get_address_space(dev), addr, buf, len,
>                            dir, attrs);
>   }
> @@ -292,6 +294,8 @@ static inline MemTxResult pci_dma_write(PCIDevice *dev, dma_addr_t addr,
>                                                  uint##_bits##_t *val, \
>                                                  MemTxAttrs attrs) \
>       { \
> +        attrs.unspecified = 0; \
> +        attrs.requester_id = pci_requester_id(dev); \
>           return ld##_l##_dma(pci_get_address_space(dev), addr, val, attrs); \
>       } \
>       static inline MemTxResult st##_s##_pci_dma(PCIDevice *dev, \
> @@ -299,6 +303,8 @@ static inline MemTxResult pci_dma_write(PCIDevice *dev, dma_addr_t addr,
>                                                  uint##_bits##_t val, \
>                                                  MemTxAttrs attrs) \
>       { \
> +        attrs.unspecified = 0; \
> +        attrs.requester_id = pci_requester_id(dev); \
>           return st##_s##_dma(pci_get_address_space(dev), addr, val, attrs); \
>       }
>   
> @@ -327,8 +333,8 @@ PCI_DMA_DEFINE_LDST(q_be, q_be, 64);
>   static inline void *pci_dma_map(PCIDevice *dev, dma_addr_t addr,
>                                   dma_addr_t *plen, DMADirection dir)
>   {
> -    return dma_memory_map(pci_get_address_space(dev), addr, plen, dir,
> -                          MEMTXATTRS_UNSPECIFIED);
> +    MemTxAttrs attrs = {.requester_id = pci_requester_id(dev)};
> +    return dma_memory_map(pci_get_address_space(dev), addr, plen, dir, attrs);
>   }
>   
>   static inline void pci_dma_unmap(PCIDevice *dev, void *buffer, dma_addr_t len,

Re: [PATCH 1/3] include/hw/pci: Attach BDF to Memory Attributes

[Jason Chien]

[-- Attachment #1: Type: text/plain, Size: 2911 bytes --]

Ping.

Michael/Marcel, would you mind taking a look? Thanks!


Jason

Daniel Henrique Barboza <dbarboza@ventanamicro.com> 於 2025年3月7日 週五 下午8:40寫道：

>
>
> On 3/2/25 6:12 AM, Jason Chien wrote:
> > This commit adds the BDF to the memory attributes for DMA operations.
> >
> > Signed-off-by: Jason Chien <jason.chien@sifive.com>
> > ---
>
> This looks sensible but I'll feel more comfortable if Michael/Marcel also
> takes a look. Thanks,
>
>
> Daniel
>
> >   include/hw/pci/pci_device.h | 10 ++++++++--
> >   1 file changed, 8 insertions(+), 2 deletions(-)
> >
> > diff --git a/include/hw/pci/pci_device.h b/include/hw/pci/pci_device.h
> > index add208edfa..968f1ba3e9 100644
> > --- a/include/hw/pci/pci_device.h
> > +++ b/include/hw/pci/pci_device.h
> > @@ -244,6 +244,8 @@ static inline MemTxResult pci_dma_rw(PCIDevice *dev,
> dma_addr_t addr,
> >                                        void *buf, dma_addr_t len,
> >                                        DMADirection dir, MemTxAttrs
> attrs)
> >   {
> > +    attrs.unspecified = 0;
> > +    attrs.requester_id = pci_requester_id(dev);
> >       return dma_memory_rw(pci_get_address_space(dev), addr, buf, len,
> >                            dir, attrs);
> >   }
> > @@ -292,6 +294,8 @@ static inline MemTxResult pci_dma_write(PCIDevice
> *dev, dma_addr_t addr,
> >                                                  uint##_bits##_t *val, \
> >                                                  MemTxAttrs attrs) \
> >       { \
> > +        attrs.unspecified = 0; \
> > +        attrs.requester_id = pci_requester_id(dev); \
> >           return ld##_l##_dma(pci_get_address_space(dev), addr, val,
> attrs); \
> >       } \
> >       static inline MemTxResult st##_s##_pci_dma(PCIDevice *dev, \
> > @@ -299,6 +303,8 @@ static inline MemTxResult pci_dma_write(PCIDevice
> *dev, dma_addr_t addr,
> >                                                  uint##_bits##_t val, \
> >                                                  MemTxAttrs attrs) \
> >       { \
> > +        attrs.unspecified = 0; \
> > +        attrs.requester_id = pci_requester_id(dev); \
> >           return st##_s##_dma(pci_get_address_space(dev), addr, val,
> attrs); \
> >       }
> >
> > @@ -327,8 +333,8 @@ PCI_DMA_DEFINE_LDST(q_be, q_be, 64);
> >   static inline void *pci_dma_map(PCIDevice *dev, dma_addr_t addr,
> >                                   dma_addr_t *plen, DMADirection dir)
> >   {
> > -    return dma_memory_map(pci_get_address_space(dev), addr, plen, dir,
> > -                          MEMTXATTRS_UNSPECIFIED);
> > +    MemTxAttrs attrs = {.requester_id = pci_requester_id(dev)};
> > +    return dma_memory_map(pci_get_address_space(dev), addr, plen, dir,
> attrs);
> >   }
> >
> >   static inline void pci_dma_unmap(PCIDevice *dev, void *buffer,
> dma_addr_t len,
>
>

[-- Attachment #2: Type: text/html, Size: 3816 bytes --]

Re: [PATCH 1/3] include/hw/pci: Attach BDF to Memory Attributes

[Jason Chien]

[-- Attachment #1: Type: text/plain, Size: 3093 bytes --]

Ping

Jason Chien <jason.chien@sifive.com> 於 2025年3月13日 週四 上午12:59寫道：

> Ping.
>
> Michael/Marcel, would you mind taking a look? Thanks!
>
>
> Jason
>
> Daniel Henrique Barboza <dbarboza@ventanamicro.com> 於 2025年3月7日 週五
> 下午8:40寫道：
>
>>
>>
>> On 3/2/25 6:12 AM, Jason Chien wrote:
>> > This commit adds the BDF to the memory attributes for DMA operations.
>> >
>> > Signed-off-by: Jason Chien <jason.chien@sifive.com>
>> > ---
>>
>> This looks sensible but I'll feel more comfortable if Michael/Marcel also
>> takes a look. Thanks,
>>
>>
>> Daniel
>>
>> >   include/hw/pci/pci_device.h | 10 ++++++++--
>> >   1 file changed, 8 insertions(+), 2 deletions(-)
>> >
>> > diff --git a/include/hw/pci/pci_device.h b/include/hw/pci/pci_device.h
>> > index add208edfa..968f1ba3e9 100644
>> > --- a/include/hw/pci/pci_device.h
>> > +++ b/include/hw/pci/pci_device.h
>> > @@ -244,6 +244,8 @@ static inline MemTxResult pci_dma_rw(PCIDevice
>> *dev, dma_addr_t addr,
>> >                                        void *buf, dma_addr_t len,
>> >                                        DMADirection dir, MemTxAttrs
>> attrs)
>> >   {
>> > +    attrs.unspecified = 0;
>> > +    attrs.requester_id = pci_requester_id(dev);
>> >       return dma_memory_rw(pci_get_address_space(dev), addr, buf, len,
>> >                            dir, attrs);
>> >   }
>> > @@ -292,6 +294,8 @@ static inline MemTxResult pci_dma_write(PCIDevice
>> *dev, dma_addr_t addr,
>> >                                                  uint##_bits##_t *val, \
>> >                                                  MemTxAttrs attrs) \
>> >       { \
>> > +        attrs.unspecified = 0; \
>> > +        attrs.requester_id = pci_requester_id(dev); \
>> >           return ld##_l##_dma(pci_get_address_space(dev), addr, val,
>> attrs); \
>> >       } \
>> >       static inline MemTxResult st##_s##_pci_dma(PCIDevice *dev, \
>> > @@ -299,6 +303,8 @@ static inline MemTxResult pci_dma_write(PCIDevice
>> *dev, dma_addr_t addr,
>> >                                                  uint##_bits##_t val, \
>> >                                                  MemTxAttrs attrs) \
>> >       { \
>> > +        attrs.unspecified = 0; \
>> > +        attrs.requester_id = pci_requester_id(dev); \
>> >           return st##_s##_dma(pci_get_address_space(dev), addr, val,
>> attrs); \
>> >       }
>> >
>> > @@ -327,8 +333,8 @@ PCI_DMA_DEFINE_LDST(q_be, q_be, 64);
>> >   static inline void *pci_dma_map(PCIDevice *dev, dma_addr_t addr,
>> >                                   dma_addr_t *plen, DMADirection dir)
>> >   {
>> > -    return dma_memory_map(pci_get_address_space(dev), addr, plen, dir,
>> > -                          MEMTXATTRS_UNSPECIFIED);
>> > +    MemTxAttrs attrs = {.requester_id = pci_requester_id(dev)};
>> > +    return dma_memory_map(pci_get_address_space(dev), addr, plen, dir,
>> attrs);
>> >   }
>> >
>> >   static inline void pci_dma_unmap(PCIDevice *dev, void *buffer,
>> dma_addr_t len,
>>
>>

[-- Attachment #2: Type: text/html, Size: 4207 bytes --]

Re: [PATCH 1/3] include/hw/pci: Attach BDF to Memory Attributes

[Jason Chien]

[-- Attachment #1: Type: text/plain, Size: 3281 bytes --]

Ping.

Jason Chien <jason.chien@sifive.com> 於 2025年3月20日 週四 上午12:40寫道：

> Ping
>
> Jason Chien <jason.chien@sifive.com> 於 2025年3月13日 週四 上午12:59寫道：
>
>> Ping.
>>
>> Michael/Marcel, would you mind taking a look? Thanks!
>>
>>
>> Jason
>>
>> Daniel Henrique Barboza <dbarboza@ventanamicro.com> 於 2025年3月7日 週五
>> 下午8:40寫道：
>>
>>>
>>>
>>> On 3/2/25 6:12 AM, Jason Chien wrote:
>>> > This commit adds the BDF to the memory attributes for DMA operations.
>>> >
>>> > Signed-off-by: Jason Chien <jason.chien@sifive.com>
>>> > ---
>>>
>>> This looks sensible but I'll feel more comfortable if Michael/Marcel also
>>> takes a look. Thanks,
>>>
>>>
>>> Daniel
>>>
>>> >   include/hw/pci/pci_device.h | 10 ++++++++--
>>> >   1 file changed, 8 insertions(+), 2 deletions(-)
>>> >
>>> > diff --git a/include/hw/pci/pci_device.h b/include/hw/pci/pci_device.h
>>> > index add208edfa..968f1ba3e9 100644
>>> > --- a/include/hw/pci/pci_device.h
>>> > +++ b/include/hw/pci/pci_device.h
>>> > @@ -244,6 +244,8 @@ static inline MemTxResult pci_dma_rw(PCIDevice
>>> *dev, dma_addr_t addr,
>>> >                                        void *buf, dma_addr_t len,
>>> >                                        DMADirection dir, MemTxAttrs
>>> attrs)
>>> >   {
>>> > +    attrs.unspecified = 0;
>>> > +    attrs.requester_id = pci_requester_id(dev);
>>> >       return dma_memory_rw(pci_get_address_space(dev), addr, buf, len,
>>> >                            dir, attrs);
>>> >   }
>>> > @@ -292,6 +294,8 @@ static inline MemTxResult pci_dma_write(PCIDevice
>>> *dev, dma_addr_t addr,
>>> >                                                  uint##_bits##_t *val,
>>> \
>>> >                                                  MemTxAttrs attrs) \
>>> >       { \
>>> > +        attrs.unspecified = 0; \
>>> > +        attrs.requester_id = pci_requester_id(dev); \
>>> >           return ld##_l##_dma(pci_get_address_space(dev), addr, val,
>>> attrs); \
>>> >       } \
>>> >       static inline MemTxResult st##_s##_pci_dma(PCIDevice *dev, \
>>> > @@ -299,6 +303,8 @@ static inline MemTxResult pci_dma_write(PCIDevice
>>> *dev, dma_addr_t addr,
>>> >                                                  uint##_bits##_t val, \
>>> >                                                  MemTxAttrs attrs) \
>>> >       { \
>>> > +        attrs.unspecified = 0; \
>>> > +        attrs.requester_id = pci_requester_id(dev); \
>>> >           return st##_s##_dma(pci_get_address_space(dev), addr, val,
>>> attrs); \
>>> >       }
>>> >
>>> > @@ -327,8 +333,8 @@ PCI_DMA_DEFINE_LDST(q_be, q_be, 64);
>>> >   static inline void *pci_dma_map(PCIDevice *dev, dma_addr_t addr,
>>> >                                   dma_addr_t *plen, DMADirection dir)
>>> >   {
>>> > -    return dma_memory_map(pci_get_address_space(dev), addr, plen, dir,
>>> > -                          MEMTXATTRS_UNSPECIFIED);
>>> > +    MemTxAttrs attrs = {.requester_id = pci_requester_id(dev)};
>>> > +    return dma_memory_map(pci_get_address_space(dev), addr, plen,
>>> dir, attrs);
>>> >   }
>>> >
>>> >   static inline void pci_dma_unmap(PCIDevice *dev, void *buffer,
>>> dma_addr_t len,
>>>
>>>

[-- Attachment #2: Type: text/html, Size: 4599 bytes --]

Re: [PATCH 1/3] include/hw/pci: Attach BDF to Memory Attributes

[Michael S. Tsirkin]

On Sun, Mar 02, 2025 at 05:12:07PM +0800, Jason Chien wrote:
> This commit adds the BDF to the memory attributes for DMA operations.
> 
> Signed-off-by: Jason Chien <jason.chien@sifive.com>
> ---
>  include/hw/pci/pci_device.h | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/include/hw/pci/pci_device.h b/include/hw/pci/pci_device.h
> index add208edfa..968f1ba3e9 100644
> --- a/include/hw/pci/pci_device.h
> +++ b/include/hw/pci/pci_device.h
> @@ -244,6 +244,8 @@ static inline MemTxResult pci_dma_rw(PCIDevice *dev, dma_addr_t addr,
>                                       void *buf, dma_addr_t len,
>                                       DMADirection dir, MemTxAttrs attrs)
>  {
> +    attrs.unspecified = 0;
> +    attrs.requester_id = pci_requester_id(dev);
>      return dma_memory_rw(pci_get_address_space(dev), addr, buf, len,
>                           dir, attrs);
>  }
> @@ -292,6 +294,8 @@ static inline MemTxResult pci_dma_write(PCIDevice *dev, dma_addr_t addr,
>                                                 uint##_bits##_t *val, \
>                                                 MemTxAttrs attrs) \
>      { \
> +        attrs.unspecified = 0; \
> +        attrs.requester_id = pci_requester_id(dev); \
>          return ld##_l##_dma(pci_get_address_space(dev), addr, val, attrs); \
>      } \
>      static inline MemTxResult st##_s##_pci_dma(PCIDevice *dev, \
> @@ -299,6 +303,8 @@ static inline MemTxResult pci_dma_write(PCIDevice *dev, dma_addr_t addr,
>                                                 uint##_bits##_t val, \
>                                                 MemTxAttrs attrs) \
>      { \
> +        attrs.unspecified = 0; \
> +        attrs.requester_id = pci_requester_id(dev); \
>          return st##_s##_dma(pci_get_address_space(dev), addr, val, attrs); \
>      }
>  
> @@ -327,8 +333,8 @@ PCI_DMA_DEFINE_LDST(q_be, q_be, 64);
>  static inline void *pci_dma_map(PCIDevice *dev, dma_addr_t addr,
>                                  dma_addr_t *plen, DMADirection dir)
>  {
> -    return dma_memory_map(pci_get_address_space(dev), addr, plen, dir,
> -                          MEMTXATTRS_UNSPECIFIED);
> +    MemTxAttrs attrs = {.requester_id = pci_requester_id(dev)};
> +    return dma_memory_map(pci_get_address_space(dev), addr, plen, dir, attrs);
>  }


Map is the only issue  - bdf can technically change between map and
unmap.
The use in hw/net/net_tx_pkt.c is fine as it's under BQL.
I don't know about the use in megasas though.
I think it is probably fine as it seems to deal with commands
and I think any driver would flush these if changing BDF.
Cc megasas maintainers just to make sure though.

Also, adding a code comment here can't hurt.


>  static inline void pci_dma_unmap(PCIDevice *dev, void *buffer, dma_addr_t len,
> -- 
> 2.43.2

Re: [PATCH 1/3] include/hw/pci: Attach BDF to Memory Attributes

[Jason Chien]

[-- Attachment #1: Type: text/plain, Size: 3387 bytes --]

Adding requester_id here does not break anything, since pci_dma_map()
passes MEMTXATTRS_UNSPECIFIED to dma_memory_map() and requester_id is
unused.

I'll add the below for the comment:
Attach BDF here for use during subsequent IOMMU translation.

Michael S. Tsirkin <mst@redhat.com> 於 2025年4月14日 週一 下午11:28寫道：

> On Sun, Mar 02, 2025 at 05:12:07PM +0800, Jason Chien wrote:
> > This commit adds the BDF to the memory attributes for DMA operations.
> >
> > Signed-off-by: Jason Chien <jason.chien@sifive.com>
> > ---
> >  include/hw/pci/pci_device.h | 10 ++++++++--
> >  1 file changed, 8 insertions(+), 2 deletions(-)
> >
> > diff --git a/include/hw/pci/pci_device.h b/include/hw/pci/pci_device.h
> > index add208edfa..968f1ba3e9 100644
> > --- a/include/hw/pci/pci_device.h
> > +++ b/include/hw/pci/pci_device.h
> > @@ -244,6 +244,8 @@ static inline MemTxResult pci_dma_rw(PCIDevice *dev,
> dma_addr_t addr,
> >                                       void *buf, dma_addr_t len,
> >                                       DMADirection dir, MemTxAttrs attrs)
> >  {
> > +    attrs.unspecified = 0;
> > +    attrs.requester_id = pci_requester_id(dev);
> >      return dma_memory_rw(pci_get_address_space(dev), addr, buf, len,
> >                           dir, attrs);
> >  }
> > @@ -292,6 +294,8 @@ static inline MemTxResult pci_dma_write(PCIDevice
> *dev, dma_addr_t addr,
> >                                                 uint##_bits##_t *val, \
> >                                                 MemTxAttrs attrs) \
> >      { \
> > +        attrs.unspecified = 0; \
> > +        attrs.requester_id = pci_requester_id(dev); \
> >          return ld##_l##_dma(pci_get_address_space(dev), addr, val,
> attrs); \
> >      } \
> >      static inline MemTxResult st##_s##_pci_dma(PCIDevice *dev, \
> > @@ -299,6 +303,8 @@ static inline MemTxResult pci_dma_write(PCIDevice
> *dev, dma_addr_t addr,
> >                                                 uint##_bits##_t val, \
> >                                                 MemTxAttrs attrs) \
> >      { \
> > +        attrs.unspecified = 0; \
> > +        attrs.requester_id = pci_requester_id(dev); \
> >          return st##_s##_dma(pci_get_address_space(dev), addr, val,
> attrs); \
> >      }
> >
> > @@ -327,8 +333,8 @@ PCI_DMA_DEFINE_LDST(q_be, q_be, 64);
> >  static inline void *pci_dma_map(PCIDevice *dev, dma_addr_t addr,
> >                                  dma_addr_t *plen, DMADirection dir)
> >  {
> > -    return dma_memory_map(pci_get_address_space(dev), addr, plen, dir,
> > -                          MEMTXATTRS_UNSPECIFIED);
> > +    MemTxAttrs attrs = {.requester_id = pci_requester_id(dev)};
> > +    return dma_memory_map(pci_get_address_space(dev), addr, plen, dir,
> attrs);
> >  }
>
>
> Map is the only issue  - bdf can technically change between map and
> unmap.
> The use in hw/net/net_tx_pkt.c is fine as it's under BQL.
> I don't know about the use in megasas though.
> I think it is probably fine as it seems to deal with commands
> and I think any driver would flush these if changing BDF.
> Cc megasas maintainers just to make sure though.
>
> Also, adding a code comment here can't hurt.
>
>
> >  static inline void pci_dma_unmap(PCIDevice *dev, void *buffer,
> dma_addr_t len,
> > --
> > 2.43.2
>
>

[-- Attachment #2: Type: text/html, Size: 4298 bytes --]

[PATCH 2/3] hw/riscv/riscv-iommu: Obtain Device IDs from Memory Attributes

[Jason Chien]

The bus number of a PCIe endpoint may change after PCIe re-enumeration,
potentially causing the device ID stored in RISCVIOMMUSpace to become
outdated. This can lead to an incorrect Device Directory Table walk.

This commit ensures that the IOMMU dynamically retrieves the latest device
IDs from the memory attributes of the requester devices, ensuring accuracy.

Signed-off-by: Jason Chien <jason.chien@sifive.com>
---
 hw/riscv/riscv-iommu.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/hw/riscv/riscv-iommu.c b/hw/riscv/riscv-iommu.c
index d46beb2d64..b72ce8e6d0 100644
--- a/hw/riscv/riscv-iommu.c
+++ b/hw/riscv/riscv-iommu.c
@@ -2644,7 +2644,13 @@ void riscv_iommu_pci_setup_iommu(RISCVIOMMUState *iommu, PCIBus *bus,
 static int riscv_iommu_memory_region_index(IOMMUMemoryRegion *iommu_mr,
     MemTxAttrs attrs)
 {
-    return attrs.unspecified ? RISCV_IOMMU_NOPROCID : (int)attrs.pid;
+    RISCVIOMMUSpace *as = container_of(iommu_mr, RISCVIOMMUSpace, iova_mr);
+
+    /* Requesters must attach its device ID. */
+    g_assert(attrs.unspecified == 0);
+
+    as->devid = attrs.requester_id;
+    return attrs.pid;
 }
 
 static int riscv_iommu_memory_region_index_len(IOMMUMemoryRegion *iommu_mr)
-- 
2.43.2

Re: [PATCH 2/3] hw/riscv/riscv-iommu: Obtain Device IDs from Memory Attributes

[Daniel Henrique Barboza]

On 3/2/25 6:12 AM, Jason Chien wrote:
> The bus number of a PCIe endpoint may change after PCIe re-enumeration,
> potentially causing the device ID stored in RISCVIOMMUSpace to become
> outdated. This can lead to an incorrect Device Directory Table walk.
> 
> This commit ensures that the IOMMU dynamically retrieves the latest device
> IDs from the memory attributes of the requester devices, ensuring accuracy.
> 
> Signed-off-by: Jason Chien <jason.chien@sifive.com>
> ---

Reviewed-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>

>   hw/riscv/riscv-iommu.c | 8 +++++++-
>   1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/riscv/riscv-iommu.c b/hw/riscv/riscv-iommu.c
> index d46beb2d64..b72ce8e6d0 100644
> --- a/hw/riscv/riscv-iommu.c
> +++ b/hw/riscv/riscv-iommu.c
> @@ -2644,7 +2644,13 @@ void riscv_iommu_pci_setup_iommu(RISCVIOMMUState *iommu, PCIBus *bus,
>   static int riscv_iommu_memory_region_index(IOMMUMemoryRegion *iommu_mr,
>       MemTxAttrs attrs)
>   {
> -    return attrs.unspecified ? RISCV_IOMMU_NOPROCID : (int)attrs.pid;
> +    RISCVIOMMUSpace *as = container_of(iommu_mr, RISCVIOMMUSpace, iova_mr);
> +
> +    /* Requesters must attach its device ID. */
> +    g_assert(attrs.unspecified == 0);
> +
> +    as->devid = attrs.requester_id;
> +    return attrs.pid;
>   }
>   
>   static int riscv_iommu_memory_region_index_len(IOMMUMemoryRegion *iommu_mr)

[PATCH 3/3] hw/riscv/riscv_iommu: Remove the "bus" property

[Jason Chien]

This property was originally intended to set the bus number for non-root
endpoints. However, since the PCIe bus number is assigned and modified
at runtime, setting this property before software execution is incorrect.
Additionally, the property incorrectly assumes that all endpoints share
the same bus, whereas no such restriction exists.

With the IOMMU now retrieving the latest device IDs from memory attributes,
there is no longer a need to set or update device IDs.

Signed-off-by: Jason Chien <jason.chien@sifive.com>
---
 hw/riscv/riscv-iommu.c | 7 -------
 hw/riscv/riscv-iommu.h | 1 -
 2 files changed, 8 deletions(-)

diff --git a/hw/riscv/riscv-iommu.c b/hw/riscv/riscv-iommu.c
index b72ce8e6d0..1ca85b95ac 100644
--- a/hw/riscv/riscv-iommu.c
+++ b/hw/riscv/riscv-iommu.c
@@ -1197,9 +1197,6 @@ static AddressSpace *riscv_iommu_space(RISCVIOMMUState *s, uint32_t devid)
 {
     RISCVIOMMUSpace *as;
 
-    /* FIXME: PCIe bus remapping for attached endpoints. */
-    devid |= s->bus << 8;
-
     QLIST_FOREACH(as, &s->spaces, list) {
         if (as->devid == devid) {
             break;
@@ -2261,9 +2258,6 @@ static MemTxResult riscv_iommu_trap_write(void *opaque, hwaddr addr,
         return MEMTX_ACCESS_ERROR;
     }
 
-    /* FIXME: PCIe bus remapping for attached endpoints. */
-    devid |= s->bus << 8;
-
     ctx = riscv_iommu_ctx(s, devid, 0, &ref);
     if (ctx == NULL) {
         res = MEMTX_ACCESS_ERROR;
@@ -2498,7 +2492,6 @@ void riscv_iommu_reset(RISCVIOMMUState *s)
 static const Property riscv_iommu_properties[] = {
     DEFINE_PROP_UINT32("version", RISCVIOMMUState, version,
         RISCV_IOMMU_SPEC_DOT_VER),
-    DEFINE_PROP_UINT32("bus", RISCVIOMMUState, bus, 0x0),
     DEFINE_PROP_UINT32("ioatc-limit", RISCVIOMMUState, iot_limit,
         LIMIT_CACHE_IOT),
     DEFINE_PROP_BOOL("intremap", RISCVIOMMUState, enable_msi, TRUE),
diff --git a/hw/riscv/riscv-iommu.h b/hw/riscv/riscv-iommu.h
index a31aa62144..655c0e71a8 100644
--- a/hw/riscv/riscv-iommu.h
+++ b/hw/riscv/riscv-iommu.h
@@ -34,7 +34,6 @@ struct RISCVIOMMUState {
     /*< public >*/
     uint32_t version;     /* Reported interface version number */
     uint32_t pid_bits;    /* process identifier width */
-    uint32_t bus;         /* PCI bus mapping for non-root endpoints */
 
     uint64_t cap;         /* IOMMU supported capabilities */
     uint64_t fctl;        /* IOMMU enabled features */
-- 
2.43.2

Re: [PATCH 3/3] hw/riscv/riscv_iommu: Remove the "bus" property

[Daniel Henrique Barboza]

On 3/2/25 6:12 AM, Jason Chien wrote:
> This property was originally intended to set the bus number for non-root
> endpoints. However, since the PCIe bus number is assigned and modified
> at runtime, setting this property before software execution is incorrect.
> Additionally, the property incorrectly assumes that all endpoints share
> the same bus, whereas no such restriction exists.
> 
> With the IOMMU now retrieving the latest device IDs from memory attributes,
> there is no longer a need to set or update device IDs.
> 
> Signed-off-by: Jason Chien <jason.chien@sifive.com>
> ---

Reviewed-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>

>   hw/riscv/riscv-iommu.c | 7 -------
>   hw/riscv/riscv-iommu.h | 1 -
>   2 files changed, 8 deletions(-)
> 
> diff --git a/hw/riscv/riscv-iommu.c b/hw/riscv/riscv-iommu.c
> index b72ce8e6d0..1ca85b95ac 100644
> --- a/hw/riscv/riscv-iommu.c
> +++ b/hw/riscv/riscv-iommu.c
> @@ -1197,9 +1197,6 @@ static AddressSpace *riscv_iommu_space(RISCVIOMMUState *s, uint32_t devid)
>   {
>       RISCVIOMMUSpace *as;
>   
> -    /* FIXME: PCIe bus remapping for attached endpoints. */
> -    devid |= s->bus << 8;
> -
>       QLIST_FOREACH(as, &s->spaces, list) {
>           if (as->devid == devid) {
>               break;
> @@ -2261,9 +2258,6 @@ static MemTxResult riscv_iommu_trap_write(void *opaque, hwaddr addr,
>           return MEMTX_ACCESS_ERROR;
>       }
>   
> -    /* FIXME: PCIe bus remapping for attached endpoints. */
> -    devid |= s->bus << 8;
> -
>       ctx = riscv_iommu_ctx(s, devid, 0, &ref);
>       if (ctx == NULL) {
>           res = MEMTX_ACCESS_ERROR;
> @@ -2498,7 +2492,6 @@ void riscv_iommu_reset(RISCVIOMMUState *s)
>   static const Property riscv_iommu_properties[] = {
>       DEFINE_PROP_UINT32("version", RISCVIOMMUState, version,
>           RISCV_IOMMU_SPEC_DOT_VER),
> -    DEFINE_PROP_UINT32("bus", RISCVIOMMUState, bus, 0x0),
>       DEFINE_PROP_UINT32("ioatc-limit", RISCVIOMMUState, iot_limit,
>           LIMIT_CACHE_IOT),
>       DEFINE_PROP_BOOL("intremap", RISCVIOMMUState, enable_msi, TRUE),
> diff --git a/hw/riscv/riscv-iommu.h b/hw/riscv/riscv-iommu.h
> index a31aa62144..655c0e71a8 100644
> --- a/hw/riscv/riscv-iommu.h
> +++ b/hw/riscv/riscv-iommu.h
> @@ -34,7 +34,6 @@ struct RISCVIOMMUState {
>       /*< public >*/
>       uint32_t version;     /* Reported interface version number */
>       uint32_t pid_bits;    /* process identifier width */
> -    uint32_t bus;         /* PCI bus mapping for non-root endpoints */
>   
>       uint64_t cap;         /* IOMMU supported capabilities */
>       uint64_t fctl;        /* IOMMU enabled features */

Re: [PATCH 3/3] hw/riscv/riscv_iommu: Remove the "bus" property

[Alistair Francis]

On Sun, Mar 2, 2025 at 7:13 PM Jason Chien <jason.chien@sifive.com> wrote:
>
> This property was originally intended to set the bus number for non-root
> endpoints. However, since the PCIe bus number is assigned and modified
> at runtime, setting this property before software execution is incorrect.
> Additionally, the property incorrectly assumes that all endpoints share
> the same bus, whereas no such restriction exists.
>
> With the IOMMU now retrieving the latest device IDs from memory attributes,
> there is no longer a need to set or update device IDs.
>
> Signed-off-by: Jason Chien <jason.chien@sifive.com>

Acked-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  hw/riscv/riscv-iommu.c | 7 -------
>  hw/riscv/riscv-iommu.h | 1 -
>  2 files changed, 8 deletions(-)
>
> diff --git a/hw/riscv/riscv-iommu.c b/hw/riscv/riscv-iommu.c
> index b72ce8e6d0..1ca85b95ac 100644
> --- a/hw/riscv/riscv-iommu.c
> +++ b/hw/riscv/riscv-iommu.c
> @@ -1197,9 +1197,6 @@ static AddressSpace *riscv_iommu_space(RISCVIOMMUState *s, uint32_t devid)
>  {
>      RISCVIOMMUSpace *as;
>
> -    /* FIXME: PCIe bus remapping for attached endpoints. */
> -    devid |= s->bus << 8;
> -
>      QLIST_FOREACH(as, &s->spaces, list) {
>          if (as->devid == devid) {
>              break;
> @@ -2261,9 +2258,6 @@ static MemTxResult riscv_iommu_trap_write(void *opaque, hwaddr addr,
>          return MEMTX_ACCESS_ERROR;
>      }
>
> -    /* FIXME: PCIe bus remapping for attached endpoints. */
> -    devid |= s->bus << 8;
> -
>      ctx = riscv_iommu_ctx(s, devid, 0, &ref);
>      if (ctx == NULL) {
>          res = MEMTX_ACCESS_ERROR;
> @@ -2498,7 +2492,6 @@ void riscv_iommu_reset(RISCVIOMMUState *s)
>  static const Property riscv_iommu_properties[] = {
>      DEFINE_PROP_UINT32("version", RISCVIOMMUState, version,
>          RISCV_IOMMU_SPEC_DOT_VER),
> -    DEFINE_PROP_UINT32("bus", RISCVIOMMUState, bus, 0x0),
>      DEFINE_PROP_UINT32("ioatc-limit", RISCVIOMMUState, iot_limit,
>          LIMIT_CACHE_IOT),
>      DEFINE_PROP_BOOL("intremap", RISCVIOMMUState, enable_msi, TRUE),
> diff --git a/hw/riscv/riscv-iommu.h b/hw/riscv/riscv-iommu.h
> index a31aa62144..655c0e71a8 100644
> --- a/hw/riscv/riscv-iommu.h
> +++ b/hw/riscv/riscv-iommu.h
> @@ -34,7 +34,6 @@ struct RISCVIOMMUState {
>      /*< public >*/
>      uint32_t version;     /* Reported interface version number */
>      uint32_t pid_bits;    /* process identifier width */
> -    uint32_t bus;         /* PCI bus mapping for non-root endpoints */
>
>      uint64_t cap;         /* IOMMU supported capabilities */
>      uint64_t fctl;        /* IOMMU enabled features */
> --
> 2.43.2
>
>