Re: [PATCH v2 2/2] vfio/common: Add trace point when a MMIO RAM section less than PAGE_SIZE

[Kunkun Jiang]

Hi Eric,

On 2021/10/22 1:02, Eric Auger wrote:

Hi Kunkun,

On 9/14/21 3:53 AM, Kunkun Jiang wrote:

The MSI-X structures of some devices and other non-MSI-X structures
are in the same BAR. They may share one host page, especially in the

may be in the same bar?

You are right. So embarrassing.

case of large page granularity, such as 64K.

For example, MSIX-Table size of 82599 NIC is 0x30 and the offset in
Bar 3(size 64KB) is 0x0. If host page size is 64KB.

remove the '.'

s/./, ?

In that case wouldn't the symptom be the same with a 4kB page?

Host page size is different, iova is different.

iova = REAL_HOST_PAGE_ALIGN(section->offset_within_address_space);


If host page size is 4KB, the iova will smaller than llend.
If host page size is 64KB, the iova will be equal to llend.


vfio_listener_region_add() will be called to map the remaining range
(0x30-0x). And it will return early at
'int128_ge((int128_make64(iova), llend))' and hasn't any message.

s/and hasn't any message /without any message


Ok, I will correct this in next version.


Let's add a trace point to informed users like commit 5c08600547c0

s/informed/inform

Same for this.

("vfio: Use a trace point when a RAM section cannot be DMA mapped")
did.

Signed-off-by: Kunkun Jiang 
---
  hw/vfio/common.c | 7 +++
  1 file changed, 7 insertions(+)

diff --git a/hw/vfio/common.c b/hw/vfio/common.c
index 8728d4d5c2..2fc6213c0f 100644
--- a/hw/vfio/common.c
+++ b/hw/vfio/common.c
@@ -892,6 +892,13 @@ static void vfio_listener_region_add(MemoryListener 
*listener,
  llend = int128_and(llend, int128_exts64(qemu_real_host_page_mask));
  
  if (int128_ge(int128_make64(iova), llend)) {

+if (memory_region_is_ram_device(section->mr)) {
+trace_vfio_listener_region_add_no_dma_map(
+memory_region_name(section->mr),
+section->offset_within_address_space,
+int128_getlo(section->size),
+qemu_real_host_page_size);
+}
  return;
  }
  end = int128_get64(int128_sub(llend, int128_one()));

By the way, if this patch is accepted. The following code in the
vfio_listener_region_add may need to be deleted:

    if (memory_region_is_ram_device(section->mr)) {
    hwaddr pgmask = (1ULL << ctz64(hostwin->iova_pgsizes)) - 1;

    if ((iova & pgmask) || (int128_get64(llsize) & pgmask)) {
    trace_vfio_listener_region_add_no_dma_map(
    memory_region_name(section->mr),
    section->offset_within_address_space,
    int128_getlo(section->size),
    pgmask + 1);
    return;
    }
    }

What do you think?

Thanks,
Kunkun Jiang

Thanks

Eric

.

Re: [PATCH v2 2/2] vfio/common: Add trace point when a MMIO RAM section less than PAGE_SIZE

[Eric Auger]

Hi Kunkun,

On 9/14/21 3:53 AM, Kunkun Jiang wrote:
> The MSI-X structures of some devices and other non-MSI-X structures
> are in the same BAR. They may share one host page, especially in the
may be in the same bar?
> case of large page granularity, such as 64K.
>
> For example, MSIX-Table size of 82599 NIC is 0x30 and the offset in
> Bar 3(size 64KB) is 0x0. If host page size is 64KB.
remove the '.'
In that case wouldn't the symptom be the same with a 4kB page?
> vfio_listener_region_add() will be called to map the remaining range
> (0x30-0x). And it will return early at
> 'int128_ge((int128_make64(iova), llend))' and hasn't any message.
s/and hasn't any message /without any message
> Let's add a trace point to informed users like commit 5c08600547c0
s/informed/inform
> ("vfio: Use a trace point when a RAM section cannot be DMA mapped")
> did.
>
> Signed-off-by: Kunkun Jiang 
> ---
>  hw/vfio/common.c | 7 +++
>  1 file changed, 7 insertions(+)
>
> diff --git a/hw/vfio/common.c b/hw/vfio/common.c
> index 8728d4d5c2..2fc6213c0f 100644
> --- a/hw/vfio/common.c
> +++ b/hw/vfio/common.c
> @@ -892,6 +892,13 @@ static void vfio_listener_region_add(MemoryListener 
> *listener,
>  llend = int128_and(llend, int128_exts64(qemu_real_host_page_mask));
>  
>  if (int128_ge(int128_make64(iova), llend)) {
> +if (memory_region_is_ram_device(section->mr)) {
> +trace_vfio_listener_region_add_no_dma_map(
> +memory_region_name(section->mr),
> +section->offset_within_address_space,
> +int128_getlo(section->size),
> +qemu_real_host_page_size);
> +}
>  return;
>  }
>  end = int128_get64(int128_sub(llend, int128_one()));
Thanks

Eric

[PATCH v2 2/2] vfio/common: Add trace point when a MMIO RAM section less than PAGE_SIZE

[Kunkun Jiang]

The MSI-X structures of some devices and other non-MSI-X structures
are in the same BAR. They may share one host page, especially in the
case of large page granularity, such as 64K.

For example, MSIX-Table size of 82599 NIC is 0x30 and the offset in
Bar 3(size 64KB) is 0x0. If host page size is 64KB.
vfio_listener_region_add() will be called to map the remaining range
(0x30-0x). And it will return early at
'int128_ge((int128_make64(iova), llend))' and hasn't any message.
Let's add a trace point to informed users like commit 5c08600547c0
("vfio: Use a trace point when a RAM section cannot be DMA mapped")
did.

Signed-off-by: Kunkun Jiang 
---
 hw/vfio/common.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/hw/vfio/common.c b/hw/vfio/common.c
index 8728d4d5c2..2fc6213c0f 100644
--- a/hw/vfio/common.c
+++ b/hw/vfio/common.c
@@ -892,6 +892,13 @@ static void vfio_listener_region_add(MemoryListener 
*listener,
 llend = int128_and(llend, int128_exts64(qemu_real_host_page_mask));
 
 if (int128_ge(int128_make64(iova), llend)) {
+if (memory_region_is_ram_device(section->mr)) {
+trace_vfio_listener_region_add_no_dma_map(
+memory_region_name(section->mr),
+section->offset_within_address_space,
+int128_getlo(section->size),
+qemu_real_host_page_size);
+}
 return;
 }
 end = int128_get64(int128_sub(llend, int128_one()));
-- 
2.23.0