On Thu, May 30, 2019 at 08:25:39PM +0200, Mark Kettenis wrote:
> I started implementing MSI-X support for arm64 adn tried to use re(4)
> for testing.  This failed miserably and when I tried to use MSI-X with
> re(4) on amd64 I noticed it didn't work either.
> 
> Turns out the Realtek hardware doesn't support 64-bit access to MSI-X
> table.  Reads return 0xffffffffffffffff and writes get ignored.
> Looking at the relevant documentation I noticed that the message
> address is actually split into two 32-bit "DWORD" entries.  Which in
> turn suggests that the Realtek folks have the standard on their side...
> 
> Diff below fixes the issue.  I did already add a "MAU32" define for
> this, even though it wasn't quite right ;).  I also took the
> opportunity to add the "function mask" bit that we'll probably need in
> the future if we start being clever with using multiple vectors and
> ave a need to guarantee atomicity of making changes to the MSI-X
> config.
> 
> ok?
> 

This reads ok to me. I believe the APIC address can be changed by
writing to the APICBASE MSR, and from what I can tell there's no requirement
that the base be < 4GB. I know we don't do that today, but should we write
the high bits of 'addr' instead of writing '0' to make things future proof?

Or is this just busywork?

Either way ok mlarkin.

-ml

> 
> Index: dev/pci/pcireg.h
> ===================================================================
> RCS file: /cvs/src/sys/dev/pci/pcireg.h,v
> retrieving revision 1.56
> diff -u -p -r1.56 pcireg.h
> --- dev/pci/pcireg.h  3 Aug 2018 22:18:13 -0000       1.56
> +++ dev/pci/pcireg.h  30 May 2019 18:06:39 -0000
> @@ -617,6 +617,7 @@ typedef u_int8_t pci_revision_t;
>   * Extended Message Signaled Interrups; access via capability pointer.
>   */
>  #define PCI_MSIX_MC_MSIXE    0x80000000
> +#define PCI_MSIX_MC_FM               0x40000000
>  #define PCI_MSIX_MC_TBLSZ_MASK       0x07ff0000
>  #define PCI_MSIX_MC_TBLSZ_SHIFT      16
>  #define PCI_MSIX_MC_TBLSZ(reg)       \
> @@ -626,7 +627,7 @@ typedef u_int8_t pci_revision_t;
>  #define  PCI_MSIX_TABLE_OFF  ~(PCI_MSIX_TABLE_BIR)
>  
>  #define PCI_MSIX_MA(i)               ((i) * 16 + 0)
> -#define PCI_MSIX_MAU32(i)    ((i) * 16 + 0)
> +#define PCI_MSIX_MAU32(i)    ((i) * 16 + 4)
>  #define PCI_MSIX_MD(i)               ((i) * 16 + 8)
>  #define PCI_MSIX_VC(i)               ((i) * 16 + 12)
>  #define  PCI_MSIX_VC_MASK    0x00000001
> Index: arch/amd64/pci/pci_machdep.c
> ===================================================================
> RCS file: /cvs/src/sys/arch/amd64/pci/pci_machdep.c,v
> retrieving revision 1.69
> diff -u -p -r1.69 pci_machdep.c
> --- arch/amd64/pci/pci_machdep.c      19 Aug 2018 08:23:47 -0000      1.69
> +++ arch/amd64/pci/pci_machdep.c      30 May 2019 18:06:39 -0000
> @@ -475,7 +475,8 @@ msix_addroute(struct pic *pic, struct cp
>           _bus_space_map(memt, base + offset, tblsz * 16, 0, &memh))
>               panic("%s: cannot map registers", __func__);
>  
> -     bus_space_write_8(memt, memh, PCI_MSIX_MA(entry), addr);
> +     bus_space_write_4(memt, memh, PCI_MSIX_MA(entry), addr);
> +     bus_space_write_4(memt, memh, PCI_MSIX_MAU32(entry), 0);
>       bus_space_write_4(memt, memh, PCI_MSIX_MD(entry), vec);
>       bus_space_barrier(memt, memh, PCI_MSIX_MA(entry), 16,
>           BUS_SPACE_BARRIER_WRITE);
> 

Reply via email to