> Date: Thu, 30 May 2019 12:43:17 -0700
> From: Mike Larkin <mlar...@nested.page>
> 
> 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?

Probably it is at this point.  This MSI-X implementation is
bug-compatible with MSI.  MSI has the issue that there might be
devices that only support a 32-bit address.  So in practice I think
current systems will always have the APIC at the fixed address of
0xffe00000.  That may change in the future at which point we should
review this code.

My arm64 code does what you suggest.

> > 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