Hi Hakkı and Jean,

could you please try this patch? It might probably fix your VT-d issues.

Thanks
  Ralf

On 6/19/19 6:52 PM, Ralf Ramsauer wrote:
> MSI-X vector tables hold 64-bit entries. So far, we used mmio_write64 to
> set them.
> 
> This conforms the PCI specification: "For all accesses to MSI-X Table and 
> MSI-X
> PBA fields, software must use aligned full DWORD or aligned full QWORD
> transactions; otherwise the result is undefined" (PCI Local Bus Specification
> Rev 3.0, chapter 8.1.2).
> 
> Nevertheless, some vendors don't support 64-bit writes, e.g., Broadcom 
> ethernet
> cards (BCM5720). mmio_write64 stalls, and the transfer won't happen.
> 
> Replace mmio_write64 with a wrapper mmio_write64_split that substitutes the
> 64-bit write with two 32-bit write operations. This accessor first writes the
> upper 32 bits and then the lower 32 bits.
> 
> Credits go to Jan, the root cause of this bug was found in a private off-list
> discussion.
> 
> Signed-off-by: Ralf Ramsauer <[email protected]>
> ---
> 
> Only tested on x86, untested on ARM. I lack PCI devices with MSI-X
> suppport.
> 
>  hypervisor/arch/arm-common/pci.c    |  4 ++--
>  hypervisor/arch/x86/pci.c           |  8 ++++----
>  hypervisor/include/jailhouse/mmio.h | 15 +++++++++++++++
>  3 files changed, 21 insertions(+), 6 deletions(-)
> 
> diff --git a/hypervisor/arch/arm-common/pci.c 
> b/hypervisor/arch/arm-common/pci.c
> index 60ffabb8..4f35175f 100644
> --- a/hypervisor/arch/arm-common/pci.c
> +++ b/hypervisor/arch/arm-common/pci.c
> @@ -58,8 +58,8 @@ int arch_pci_update_msi(struct pci_device *device,
>  int arch_pci_update_msix_vector(struct pci_device *device, unsigned int 
> index)
>  {
>       /* NOTE: See arch_pci_update_msi. */
> -     mmio_write64(&device->msix_table[index].address,
> -                  device->msix_vectors[index].address);
> +     mmio_write64_split(&device->msix_table[index].address,
> +                        device->msix_vectors[index].address);
>       mmio_write32(&device->msix_table[index].data,
>                    device->msix_vectors[index].data);
>       return 0;
> diff --git a/hypervisor/arch/x86/pci.c b/hypervisor/arch/x86/pci.c
> index 492e9859..560c5697 100644
> --- a/hypervisor/arch/x86/pci.c
> +++ b/hypervisor/arch/x86/pci.c
> @@ -393,8 +393,8 @@ int arch_pci_update_msix_vector(struct pci_device 
> *device, unsigned int index)
>                                    irq_msg);
>       // HACK for QEMU
>       if (result == -ENOSYS) {
> -             mmio_write64(&device->msix_table[index].address,
> -                          device->msix_vectors[index].address);
> +             mmio_write64_split(&device->msix_table[index].address,
> +                                device->msix_vectors[index].address);
>               mmio_write32(&device->msix_table[index].data,
>                            device->msix_vectors[index].data);
>               return 0;
> @@ -402,8 +402,8 @@ int arch_pci_update_msix_vector(struct pci_device 
> *device, unsigned int index)
>       if (result < 0)
>               return result;
>  
> -     mmio_write64(&device->msix_table[index].address,
> -                  pci_get_x86_msi_remap_address(result));
> +     mmio_write64_split(&device->msix_table[index].address,
> +                        pci_get_x86_msi_remap_address(result));
>       mmio_write32(&device->msix_table[index].data, 0);
>  
>       return 0;
> diff --git a/hypervisor/include/jailhouse/mmio.h 
> b/hypervisor/include/jailhouse/mmio.h
> index 61b4647e..567901d0 100644
> --- a/hypervisor/include/jailhouse/mmio.h
> +++ b/hypervisor/include/jailhouse/mmio.h
> @@ -73,6 +73,21 @@ DEFINE_MMIO_WRITE(32)
>  DEFINE_MMIO_WRITE(64)
>  /** @} */
>  
> +/**
> + * Write a 64-bit value to a memory-mapper I/O register. Perform two 32-bit
> + * write operations instead of one 64-bit write operation.
> + *
> + * @param address    Virtual address of the register.
> + * @param value              Value to write.
> + * @{
> + */
> +static inline void mmio_write64_split(void *address, u64 value)
> +{
> +     mmio_write32(address + sizeof(u32), (u32)(value >> (sizeof(u32) * 8)));
> +     mmio_write32(address, (u32)value);
> +}
> +/** @} */
> +
>  /**
>   * Read value from 32 or 64-bit MMIO register field.
>   * @param address    Virtual address of the register.
> 

-- 
You received this message because you are subscribed to the Google Groups 
"Jailhouse" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to [email protected].
To view this discussion on the web visit 
https://groups.google.com/d/msgid/jailhouse-dev/02c8a219-4be2-fb4b-6a06-e30e726dfefb%40oth-regensburg.de.
For more options, visit https://groups.google.com/d/optout.

Reply via email to