* [PATCH 0/3] Enhancing Device Identification in RISC-V IOMMU Using Memory Attributes
@ 2025-03-02 9:12 Jason Chien
2025-03-02 9:12 ` [PATCH 1/3] include/hw/pci: Attach BDF to " Jason Chien
` (2 more replies)
0 siblings, 3 replies; 13+ messages in thread
From: Jason Chien @ 2025-03-02 9:12 UTC (permalink / raw)
To: qemu-devel, qemu-riscv
Cc: Palmer Dabbelt, Alistair Francis, Weiwei Li,
Daniel Henrique Barboza, Liu Zhiwei, Michael S. Tsirkin,
Marcel Apfelbaum, 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
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 1/3] include/hw/pci: Attach BDF to Memory Attributes
2025-03-02 9:12 [PATCH 0/3] Enhancing Device Identification in RISC-V IOMMU Using Memory Attributes Jason Chien
@ 2025-03-02 9:12 ` Jason Chien
2025-03-07 12:39 ` Daniel Henrique Barboza
2025-04-14 15:28 ` Michael S. Tsirkin
2025-03-02 9:12 ` [PATCH 2/3] hw/riscv/riscv-iommu: Obtain Device IDs from " Jason Chien
2025-03-02 9:12 ` [PATCH 3/3] hw/riscv/riscv_iommu: Remove the "bus" property Jason Chien
2 siblings, 2 replies; 13+ messages in thread
From: Jason Chien @ 2025-03-02 9:12 UTC (permalink / raw)
To: qemu-devel, qemu-riscv
Cc: Palmer Dabbelt, Alistair Francis, Weiwei Li,
Daniel Henrique Barboza, Liu Zhiwei, Michael S. Tsirkin,
Marcel Apfelbaum, 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
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 2/3] hw/riscv/riscv-iommu: Obtain Device IDs from Memory Attributes
2025-03-02 9:12 [PATCH 0/3] Enhancing Device Identification in RISC-V IOMMU Using Memory Attributes Jason Chien
2025-03-02 9:12 ` [PATCH 1/3] include/hw/pci: Attach BDF to " Jason Chien
@ 2025-03-02 9:12 ` Jason Chien
2025-03-07 12:35 ` Daniel Henrique Barboza
2025-03-02 9:12 ` [PATCH 3/3] hw/riscv/riscv_iommu: Remove the "bus" property Jason Chien
2 siblings, 1 reply; 13+ messages in thread
From: Jason Chien @ 2025-03-02 9:12 UTC (permalink / raw)
To: qemu-devel, qemu-riscv
Cc: Palmer Dabbelt, Alistair Francis, Weiwei Li,
Daniel Henrique Barboza, Liu Zhiwei, Michael S. Tsirkin,
Marcel Apfelbaum, 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
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 3/3] hw/riscv/riscv_iommu: Remove the "bus" property
2025-03-02 9:12 [PATCH 0/3] Enhancing Device Identification in RISC-V IOMMU Using Memory Attributes Jason Chien
2025-03-02 9:12 ` [PATCH 1/3] include/hw/pci: Attach BDF to " Jason Chien
2025-03-02 9:12 ` [PATCH 2/3] hw/riscv/riscv-iommu: Obtain Device IDs from " Jason Chien
@ 2025-03-02 9:12 ` Jason Chien
2025-03-07 12:36 ` Daniel Henrique Barboza
2025-04-04 1:43 ` Alistair Francis
2 siblings, 2 replies; 13+ messages in thread
From: Jason Chien @ 2025-03-02 9:12 UTC (permalink / raw)
To: qemu-devel, qemu-riscv
Cc: Palmer Dabbelt, Alistair Francis, Weiwei Li,
Daniel Henrique Barboza, Liu Zhiwei, Michael S. Tsirkin,
Marcel Apfelbaum, 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
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 2/3] hw/riscv/riscv-iommu: Obtain Device IDs from Memory Attributes
2025-03-02 9:12 ` [PATCH 2/3] hw/riscv/riscv-iommu: Obtain Device IDs from " Jason Chien
@ 2025-03-07 12:35 ` Daniel Henrique Barboza
0 siblings, 0 replies; 13+ messages in thread
From: Daniel Henrique Barboza @ 2025-03-07 12:35 UTC (permalink / raw)
To: Jason Chien, qemu-devel, qemu-riscv
Cc: Palmer Dabbelt, Alistair Francis, Weiwei Li, Liu Zhiwei,
Michael S. Tsirkin, Marcel Apfelbaum
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)
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 3/3] hw/riscv/riscv_iommu: Remove the "bus" property
2025-03-02 9:12 ` [PATCH 3/3] hw/riscv/riscv_iommu: Remove the "bus" property Jason Chien
@ 2025-03-07 12:36 ` Daniel Henrique Barboza
2025-04-04 1:43 ` Alistair Francis
1 sibling, 0 replies; 13+ messages in thread
From: Daniel Henrique Barboza @ 2025-03-07 12:36 UTC (permalink / raw)
To: Jason Chien, qemu-devel, qemu-riscv
Cc: Palmer Dabbelt, Alistair Francis, Weiwei Li, Liu Zhiwei,
Michael S. Tsirkin, Marcel Apfelbaum
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 */
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/3] include/hw/pci: Attach BDF to Memory Attributes
2025-03-02 9:12 ` [PATCH 1/3] include/hw/pci: Attach BDF to " Jason Chien
@ 2025-03-07 12:39 ` Daniel Henrique Barboza
2025-03-12 16:59 ` Jason Chien
2025-04-14 15:28 ` Michael S. Tsirkin
1 sibling, 1 reply; 13+ messages in thread
From: Daniel Henrique Barboza @ 2025-03-07 12:39 UTC (permalink / raw)
To: Jason Chien, qemu-devel, qemu-riscv
Cc: Palmer Dabbelt, Alistair Francis, Weiwei Li, Liu Zhiwei,
Michael S. Tsirkin, Marcel Apfelbaum
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,
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/3] include/hw/pci: Attach BDF to Memory Attributes
2025-03-07 12:39 ` Daniel Henrique Barboza
@ 2025-03-12 16:59 ` Jason Chien
2025-03-19 16:40 ` Jason Chien
0 siblings, 1 reply; 13+ messages in thread
From: Jason Chien @ 2025-03-12 16:59 UTC (permalink / raw)
To: Daniel Henrique Barboza
Cc: qemu-devel, qemu-riscv, Palmer Dabbelt, Alistair Francis,
Weiwei Li, Liu Zhiwei, Michael S. Tsirkin, Marcel Apfelbaum
[-- 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 --]
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/3] include/hw/pci: Attach BDF to Memory Attributes
2025-03-12 16:59 ` Jason Chien
@ 2025-03-19 16:40 ` Jason Chien
2025-04-14 15:10 ` Jason Chien
0 siblings, 1 reply; 13+ messages in thread
From: Jason Chien @ 2025-03-19 16:40 UTC (permalink / raw)
To: Daniel Henrique Barboza
Cc: qemu-devel, qemu-riscv, Palmer Dabbelt, Alistair Francis,
Weiwei Li, Liu Zhiwei, Michael S. Tsirkin, Marcel Apfelbaum
[-- 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 --]
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 3/3] hw/riscv/riscv_iommu: Remove the "bus" property
2025-03-02 9:12 ` [PATCH 3/3] hw/riscv/riscv_iommu: Remove the "bus" property Jason Chien
2025-03-07 12:36 ` Daniel Henrique Barboza
@ 2025-04-04 1:43 ` Alistair Francis
1 sibling, 0 replies; 13+ messages in thread
From: Alistair Francis @ 2025-04-04 1:43 UTC (permalink / raw)
To: Jason Chien
Cc: qemu-devel, qemu-riscv, Palmer Dabbelt, Alistair Francis,
Weiwei Li, Daniel Henrique Barboza, Liu Zhiwei,
Michael S. Tsirkin, Marcel Apfelbaum
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
>
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/3] include/hw/pci: Attach BDF to Memory Attributes
2025-03-19 16:40 ` Jason Chien
@ 2025-04-14 15:10 ` Jason Chien
0 siblings, 0 replies; 13+ messages in thread
From: Jason Chien @ 2025-04-14 15:10 UTC (permalink / raw)
To: Daniel Henrique Barboza
Cc: qemu-devel, qemu-riscv, Palmer Dabbelt, Alistair Francis,
Weiwei Li, Liu Zhiwei, Michael S. Tsirkin, Marcel Apfelbaum
[-- 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 --]
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/3] include/hw/pci: Attach BDF to Memory Attributes
2025-03-02 9:12 ` [PATCH 1/3] include/hw/pci: Attach BDF to " Jason Chien
2025-03-07 12:39 ` Daniel Henrique Barboza
@ 2025-04-14 15:28 ` Michael S. Tsirkin
2025-04-24 7:57 ` Jason Chien
1 sibling, 1 reply; 13+ messages in thread
From: Michael S. Tsirkin @ 2025-04-14 15:28 UTC (permalink / raw)
To: Jason Chien
Cc: qemu-devel, qemu-riscv, Palmer Dabbelt, Alistair Francis,
Weiwei Li, Daniel Henrique Barboza, Liu Zhiwei, Marcel Apfelbaum,
Paolo Bonzini, Fam Zheng, Hannes Reinecke, qemu-block
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
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/3] include/hw/pci: Attach BDF to Memory Attributes
2025-04-14 15:28 ` Michael S. Tsirkin
@ 2025-04-24 7:57 ` Jason Chien
0 siblings, 0 replies; 13+ messages in thread
From: Jason Chien @ 2025-04-24 7:57 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: qemu-devel, qemu-riscv, Palmer Dabbelt, Alistair Francis,
Weiwei Li, Daniel Henrique Barboza, Liu Zhiwei, Marcel Apfelbaum,
Paolo Bonzini, Fam Zheng, Hannes Reinecke, qemu-block
[-- 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 --]
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2025-04-24 7:58 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-03-02 9:12 [PATCH 0/3] Enhancing Device Identification in RISC-V IOMMU Using Memory Attributes Jason Chien
2025-03-02 9:12 ` [PATCH 1/3] include/hw/pci: Attach BDF to " Jason Chien
2025-03-07 12:39 ` Daniel Henrique Barboza
2025-03-12 16:59 ` Jason Chien
2025-03-19 16:40 ` Jason Chien
2025-04-14 15:10 ` Jason Chien
2025-04-14 15:28 ` Michael S. Tsirkin
2025-04-24 7:57 ` Jason Chien
2025-03-02 9:12 ` [PATCH 2/3] hw/riscv/riscv-iommu: Obtain Device IDs from " Jason Chien
2025-03-07 12:35 ` Daniel Henrique Barboza
2025-03-02 9:12 ` [PATCH 3/3] hw/riscv/riscv_iommu: Remove the "bus" property Jason Chien
2025-03-07 12:36 ` Daniel Henrique Barboza
2025-04-04 1:43 ` Alistair Francis
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 NNTP newsgroup(s).