Re: try to map 64-bit mem space first

[Ted Unangst]

Mark Kettenis wrote:
> We should allways use mmio instead of io if possible.  The cascading
> if statements are a bit ugly, but I can't come up with a better solution.

mapped = 0;
if (pcimap() == 0)
mapped = 1;
if (!mapped && pcimap() == 0)
mapped = 1;
if (!mapped)
printf("no mapping worked");

Not a lot prettier, but avoids neverending indentation, and perhaps clarifies
that we're basically trying the same thing with different arguments instead of
descending into some bizarre error handling rabbit hole.

Re: try to map 64-bit mem space first

[Mike Larkin]

On Mon, Apr 09, 2018 at 08:27:49PM +0200, Mark Kettenis wrote:
> > Date: Mon, 9 Apr 2018 18:31:49 +0200
> > From: Patrick Wildt 
> > 
> > Hi,
> > 
> > some (probably newer) re(4) cards don't have the 32-bit memory BAR that
> > we try to map first.  Instead there's a 64-bit memory BAR in the follow-
> > ing BAR.   On the MacchiatoBIN, where we currently do not support the IO
> > space, this means we need to attempt to map the 64-bit memory BAR.  This
> > diff tries to map the 64-bit BAR first, and falls back to 32-bit BAR and
> > IO after that.  This makes re(4) on that machine.  It looks a but ugly,
> > but I don't think there's a nicer way.  Thanks to kettenis@ for finding
> > the cause of my issues!
> > 
> > ok?
> 
> We should allways use mmio instead of io if possible.  The cascading
> if statements are a bit ugly, but I can't come up with a better solution.
> 
> Would be great if somebody could test this on an older (classic PCI)
> card.
> 

Works fine here on:

re0 at pci3 dev 0 function 0 "Realtek 8168" rev 0x02: RTL8168C/8111C (0x3c00), 
msi, address 00:0a:cd:18:3c:76
rgephy0 at re0 phy 7: RTL8169S/8110S/8211 PHY, rev. 2

This card is probably 5+ years old. It has the following BAR configuration:

 4:0:0: Realtek 8168
0x: Vendor ID: 10ec Product ID: 8168
0x0004: Command: 0006 Status: 0010
0x0008: Class: 02 Subclass: 00 Interface: 00 Revision: 02
0x000c: BIST: 00 Header Type: 00 Latency Timer: 00 Cache Line Size: 00
0x0010: BAR io addr: 0x/0x0100
0x0014: BAR empty ()
0x0018: BAR mem 64bit addr: 0xf140/0x1000
0x0020: BAR mem prefetchable 64bit addr: 0x/0x0001

-ml

> ok kettenis@
> 
> > diff --git a/sys/dev/ic/rtl81x9reg.h b/sys/dev/ic/rtl81x9reg.h
> > index 8fc4a9a851d..6f7019ebdbd 100644
> > --- a/sys/dev/ic/rtl81x9reg.h
> > +++ b/sys/dev/ic/rtl81x9reg.h
> > @@ -1061,6 +1061,7 @@ struct rl_softc {
> >  #define RL_PCI_HEADER_TYPE 0x0E
> >  #define RL_PCI_LOIO0x10
> >  #define RL_PCI_LOMEM   0x14
> > +#define RL_PCI_LOMEM64 0x18
> >  #define RL_PCI_BIOSROM 0x30
> >  #define RL_PCI_INTLINE 0x3C
> >  #define RL_PCI_INTPIN  0x3D
> > diff --git a/sys/dev/pci/if_re_pci.c b/sys/dev/pci/if_re_pci.c
> > index 84d1f3f2b04..c553aa17349 100644
> > --- a/sys/dev/pci/if_re_pci.c
> > +++ b/sys/dev/pci/if_re_pci.c
> > @@ -141,12 +141,18 @@ re_pci_attach(struct device *parent, struct device 
> > *self, void *aux)
> > /*
> >  * Map control/status registers.
> >  */
> > -   if (pci_mapreg_map(pa, RL_PCI_LOMEM, PCI_MAPREG_TYPE_MEM, 0,
> > -   >rl_btag, >rl_bhandle, NULL, >sc_iosize, 0)) {
> > -   if (pci_mapreg_map(pa, RL_PCI_LOIO, PCI_MAPREG_TYPE_IO, 0,
> > -   >rl_btag, >rl_bhandle, NULL, >sc_iosize, 0)) {
> > -   printf(": can't map mem or i/o space\n");
> > -   return;
> > +   if (pci_mapreg_map(pa, RL_PCI_LOMEM64, PCI_MAPREG_TYPE_MEM |
> > +   PCI_MAPREG_MEM_TYPE_64BIT, 0, >rl_btag, >rl_bhandle,
> > +   NULL, >sc_iosize, 0)) {
> > +   if (pci_mapreg_map(pa, RL_PCI_LOMEM, PCI_MAPREG_TYPE_MEM |
> > +   PCI_MAPREG_MEM_TYPE_32BIT, 0, >rl_btag, >rl_bhandle,
> > +   NULL, >sc_iosize, 0)) {
> > +   if (pci_mapreg_map(pa, RL_PCI_LOIO, PCI_MAPREG_TYPE_IO,
> > +   0, >rl_btag, >rl_bhandle, NULL,
> > +   >sc_iosize, 0)) {
> > +   printf(": can't map mem or i/o space\n");
> > +   return;
> > +   }
> > }
> > }
> >  
> > 
> > 
> 

Re: try to map 64-bit mem space first

[Mark Kettenis]

> Date: Mon, 9 Apr 2018 18:31:49 +0200
> From: Patrick Wildt 
> 
> Hi,
> 
> some (probably newer) re(4) cards don't have the 32-bit memory BAR that
> we try to map first.  Instead there's a 64-bit memory BAR in the follow-
> ing BAR.   On the MacchiatoBIN, where we currently do not support the IO
> space, this means we need to attempt to map the 64-bit memory BAR.  This
> diff tries to map the 64-bit BAR first, and falls back to 32-bit BAR and
> IO after that.  This makes re(4) on that machine.  It looks a but ugly,
> but I don't think there's a nicer way.  Thanks to kettenis@ for finding
> the cause of my issues!
> 
> ok?

We should allways use mmio instead of io if possible.  The cascading
if statements are a bit ugly, but I can't come up with a better solution.

Would be great if somebody could test this on an older (classic PCI)
card.

ok kettenis@

> diff --git a/sys/dev/ic/rtl81x9reg.h b/sys/dev/ic/rtl81x9reg.h
> index 8fc4a9a851d..6f7019ebdbd 100644
> --- a/sys/dev/ic/rtl81x9reg.h
> +++ b/sys/dev/ic/rtl81x9reg.h
> @@ -1061,6 +1061,7 @@ struct rl_softc {
>  #define RL_PCI_HEADER_TYPE   0x0E
>  #define RL_PCI_LOIO  0x10
>  #define RL_PCI_LOMEM 0x14
> +#define RL_PCI_LOMEM64   0x18
>  #define RL_PCI_BIOSROM   0x30
>  #define RL_PCI_INTLINE   0x3C
>  #define RL_PCI_INTPIN0x3D
> diff --git a/sys/dev/pci/if_re_pci.c b/sys/dev/pci/if_re_pci.c
> index 84d1f3f2b04..c553aa17349 100644
> --- a/sys/dev/pci/if_re_pci.c
> +++ b/sys/dev/pci/if_re_pci.c
> @@ -141,12 +141,18 @@ re_pci_attach(struct device *parent, struct device 
> *self, void *aux)
>   /*
>* Map control/status registers.
>*/
> - if (pci_mapreg_map(pa, RL_PCI_LOMEM, PCI_MAPREG_TYPE_MEM, 0,
> - >rl_btag, >rl_bhandle, NULL, >sc_iosize, 0)) {
> - if (pci_mapreg_map(pa, RL_PCI_LOIO, PCI_MAPREG_TYPE_IO, 0,
> - >rl_btag, >rl_bhandle, NULL, >sc_iosize, 0)) {
> - printf(": can't map mem or i/o space\n");
> - return;
> + if (pci_mapreg_map(pa, RL_PCI_LOMEM64, PCI_MAPREG_TYPE_MEM |
> + PCI_MAPREG_MEM_TYPE_64BIT, 0, >rl_btag, >rl_bhandle,
> + NULL, >sc_iosize, 0)) {
> + if (pci_mapreg_map(pa, RL_PCI_LOMEM, PCI_MAPREG_TYPE_MEM |
> + PCI_MAPREG_MEM_TYPE_32BIT, 0, >rl_btag, >rl_bhandle,
> + NULL, >sc_iosize, 0)) {
> + if (pci_mapreg_map(pa, RL_PCI_LOIO, PCI_MAPREG_TYPE_IO,
> + 0, >rl_btag, >rl_bhandle, NULL,
> + >sc_iosize, 0)) {
> + printf(": can't map mem or i/o space\n");
> + return;
> + }
>   }
>   }
>  
> 
>