Re: [PATCH v3 3/3] Do not access /dev/mem in MSI-X PCI passthrough on Xen

[Anthony PERARD <anthony.perard@xxxxxxxxxx>]

On Mon, May 06, 2024 at 02:33:22AM +0200, Marek Marczykowski-Górecki wrote:
> diff --git a/hw/xen/xen_pt_msi.c b/hw/xen/xen_pt_msi.c
> index 09cca4e..836cc9c 100644
> --- a/hw/xen/xen_pt_msi.c
> +++ b/hw/xen/xen_pt_msi.c
> @@ -493,7 +501,12 @@ static uint64_t pci_msix_read(void *opaque, hwaddr addr,
>          return get_entry_value(&msix->msix_entry[entry_nr], offset);
>      } else {
>          /* Pending Bit Array (PBA) */
> -        return *(uint32_t *)(msix->phys_iomem_base + addr);
> +        if (s->msix->phys_iomem_base) {
> +            return *(uint32_t *)(msix->phys_iomem_base + addr);
> +        }
> +        XEN_PT_LOG(&s->dev, "reading PBA, addr 0x%lx, offset 0x%lx\n",
> +                   addr, addr - msix->total_entries * PCI_MSIX_ENTRY_SIZE);
> +        return 0xFFFFFFFF;

If Xen advertise XENFEAT_dm_msix_all_writes, we are not expecting QEMU
to reach this code, right? A comment might be useful.

>      }
>  }
>
> @@ -576,33 +593,40 @@ int xen_pt_msix_init(XenPCIPassthroughState *s, 
> uint32_t base)
>      msix->table_base = s->real_device.io_regions[bar_index].base_addr;
>      XEN_PT_LOG(d, "get MSI-X table BAR base 0x%"PRIx64"\n", 
> msix->table_base);
>
> +    /* Accessing /dev/mem is needed only on older Xen. */
> +    if (!(xc_version_info.submap & (1U << XENFEAT_dm_msix_all_writes))) {

Would it be ok to use test_bit() instead?

Thanks,

--

Anthony Perard | Vates XCP-ng Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech