Kernel code documentation and pci data structures (was: Problem with Intel WiFi card)

[Rocky Hotas]

Hi Masanobu and hi all!
After the discussion on this thread,

http://mail-index.netbsd.org/netbsd-users/2017/02/01/msg019280.html

I'm trying to apply this patch

http://cvsweb.openbsd.org/cgi-bin/cvsweb/src/sys/dev/pci/ppb.c.diff?r1=1.35&r2=1.36

to NetBSD. The current version of OpenBSD kernel ppb.c file (which
works in my laptop, where I tried to temporarily install OpenBSD)
has some more lines and is based on the rev. 1.40.
I write to you because you made much of the last commits on the
NetBSD ppb.c sourcefile; I  write also to the list, to let the
community know these updates.
OpenBSD code is based upon the following members of the
`struct pci_attach_args':

struct extent   *pa_ioex;
struct extent   *pa_memex;

They must be added to the NetBSD code, and then correctly handled.
The extent(9) manpages both in NetBSD and in OpenBSDare both too
much concise. These pages

https://www.netbsd.org/docs/internals/en/
https://www.netbsd.org/docs/kernel/

don't mention nor pci neither extents.
So, a couple of questions:

1) What are the extent(9) used for? They were born in NetBSD (as it
is even stated in the OpenBSD manpage). After a quick discussion on
#netbsd-code with coypu, they seem to be related to address spaces.
If there is some guide like this one

http://www.unixag-kl.fh-kl.de/~jkunz/projekte/NetBSD-driver_writing-1.0.1e.pdf.gz

but for extents(9) and memory, please let me know.

2) About these lines (that are both in NetBSD and in OpenBSD):

static int
ppbmatch(device_t parent, cfdata_t match, void *aux)
{
struct pci_attach_args *pa = aux;


driver(9) explicitly states that "The match function would
type-cast the aux argument to its appropriate attachment structure and
use its contents to determine whether it supports the device". But who
initially fills the `aux' data? Whoever he is, he should then take
care about some new members, like `pa_ioex' and `pa_memex'.

Thank you for having read,

Rocky

Re: Kernel code documentation and pci data structures (was: Problem with Intel WiFi card)

[Rocky Hotas]

Hi :)!

> Sent: Thursday, May 18, 2017 at 9:29 PM
> From: "Taylor R Campbell" 
> To: "Rocky Hotas" 
> Cc: msai...@netbsd.org, tech-kern@netbsd.org
> Subject: Re: Kernel code documentation and pci data structures (was: Problem 
> with Intel WiFi card)
> 
> Wild guess: Doing
> 
>   extent_alloc(pa->pa_ioex, size, alignment, boundary, flags, &addr)
> 
> in OpenBSD, where addr is an unsigned long, is like doing
> 
>   bus_space_alloc(pa->pa_iot, 0, -1, size, alignment, boundary, flags,
>   &addr, NULL)
> 
> in NetBSD, where addr is a bus_addr_t, and likewise with OpenBSD
> pa->pa_memex / NetBSD pa->pa_memt.

First of all, thank you for your answer and the "wild guess".
I remind you that the function called inside the OpenBSD
`ppb_alloc_resources' is `extent_alloc_subregion'. Maybe the NetBSD
`reg_start' and `reg_end' (type `bus_addr_t') can be equivalent of
OpenBSD `substart' and `subend' (type `ulong').
You are basically stating that in NetBSD the `extents' could be avoided,
achieving the same result? If so, remember that the OpenBSD
`struct pci_attach_args' has a member `pa_iot' too (surely
belonging to the initial NetBSD code), but it is not used in this case.
Bye!

Rocky

Re: Kernel code documentation and pci data structures (was: Problem with Intel WiFi card)

[Rocky Hotas]

Hi Masanobu and hi all!

> Sent: Tuesday, May 23, 2017 at 6:18 AM
> From: "Masanobu SAITOH" 
> To: "Rocky Hotas" , msai...@netbsd.org
> Cc: msai...@execsw.org, tech-kern@netbsd.org
> Subject: Re: Kernel code documentation and pci data structures (was: Problem 
> with Intel WiFi card)
> 
>   It's one of the thing that I want to have it in our ppb.c.
> I'd like to add PCI(e) hot-plug stuff because of nvme(4)
> hot-plug. Reasons why I've not doing the work yet are that
> I'm now working another job(fixing bugs in ixg(4) and wm(4))
> and I'm not familiar with resource allocation near here :-(

I can understand you. Moreover, it is difficult as I was saying to
get documented about it.

> I think it's worth for you to
> read sys/dev/pci/pciconf.c

Thank you, I'll try.

> Did you check the PCI config area of ppb"1" on your machine?
> iwn0 is under ppb1.

Yes and it is not configured, as expected. The base/limit registers
of the "I/O", "Memory" and "Prefetchable memory" regions are
left configured, while instead the code in OpenBSD manages them.

> Could you show me the output of "pcictl pciN dump" of your ppb1?

You can find it here: https://pastebin.com/bkwXMuxX

> And, If you are OK, could you me the full output of pcictl
> using with the following script?
> 
>   http://www.netbsd.org/~msaitoh/pcidump
> 

Yes, no problem. I had some issue to upload it, though.
These are two options:
1) http://www119.zippyshare.com/v/DUR50NZw/file.html
(It is a page from where the file can be downloaded: I tried and it
worked, but be careful: a pop-up or similar can be opened!)
2) https://www.sendspace.com/file/ddnvqd
(I tried here also, and it worked, without popups)

Both should expire after 30 days of no use.
For further information: I re-compiled the kernel today
(it is -current, version 7.99.73), with

options PCIVERBOSE
options PCI_CONFIG_DUMP
options MSGBUFSIZE=262144

and here is the content of `/var/run/dmesg.boot' about ppb1:

https://pastebin.com/zaACgZXf

(unfortunately the dmesg was not entirely recorded, despite the
huge MSGBUFSIZE, but the interesting part here has been saved).
In particular, notice that

I/O region:
  base register:  0x00
  limit register: 0x00
...
Memory region:
  base register:  0x
  limit register: 0x

Prefetchable memory region:
  base register:  0x0001
  limit register: 0x0001

as anticipated.
If it can be useful, this is the dump of Configuration Space
in Linux of the same ppb:

https://pastebin.com/pFbcMWYp
(in human-readable form)

https://pastebin.com/d3xCKjCZ
(full, in hexadecimal)

In Linux, this WiFi card (and the associated ppb) works.
Sorry for my delay, it took a while to get all these files.
Thank you for your reply!

Rocky


>   Regards.
> 
> -- 
> ---
>  SAITOH Masanobu (msai...@execsw.org
>   msai...@netbsd.org)
> 

Different reset functions in if_msk.c

[Rocky Hotas]

Hi all!
I am trying to include in the -current NetBSD src/sys/dev/pci/if_msk.c the 
OpenBSD
code modifications since OpenBSD if_msk.c rev. 1.43. This is to add a driver for
my ethernet NIC Marvell 88E8072, which is currently not supported by NetBSD.
OpenBSD, in rev. 1.48, includes a `void mskc_reset(struct sk_softc *);' line:

http://cvsweb.openbsd.org/cgi-bin/cvsweb/src/sys/dev/pci/if_msk.c.diff?r1=1.47&r2=1.48&f=h

If it's possible, I would like to make some observations and questions:

- This new line is not in the NetBSD code.
- Both NetBSD and OpenBSD have instead a `void msk_reset' function: what can be 
the
differences between `msk_reset' and `mskc_reset'?
- Moreover, they need different argument types:

OpenBSD
void mskc_reset(struct sk_softc *);
void msk_reset(struct sk_if_softc *);

NetBSD
void msk_reset(struct sk_softc *);

NetBSD msk_reset asks for a `struct sk_softc *', while OpenBSD msk_reset asks 
for a
`struct sk_if_softc *'. They seem to be amusingly swapped. So, when 
`mskc_reset' is
included in NetBSD, which will be the correct data type for its argument?
Shall I use `struct sk_softc *' as in OpenBSD or shall I blindly continue the 
"swap law"
and use a `struct sk_if_softc *'?
Bye!

Rocky

Some updates

[Rocky Hotas]

Hello again!
The function `void mskc_reset(struct sk_softc *sc)' in OpenBSD is
approximately the same as the NetBSD `void msk_reset(struct sk_softc *sc)'.
There are some swapped lines and maybe some different code blocks, but
the basic structure is almost equal.
It is invoked inside `msk_watchdog' in both the sourcecodes (even if
`msk_watchdog itself is slightly different in the two cases) and in
`mskc_shutdown', which is not present in NetBSD.

As regards the OpenBSD `msk_reset', it is the same as a bunch of lines
included in NetBSD `msk_init_yukon':

SK_IF_WRITE_4(sc_if, 0, SK_GMAC_CTRL, SK_GMAC_RESET_SET);
SK_IF_WRITE_4(sc_if, 0, SK_GPHY_CTRL, SK_GPHY_RESET_SET);
DELAY(1000);

DPRINTFN(6, ("msk_init_yukon: 2\n"));
 
SK_IF_WRITE_4(sc_if, 0, SK_GPHY_CTRL, SK_GPHY_RESET_CLEAR);
SK_IF_WRITE_4(sc_if, 0, SK_GMAC_CTRL, SK_GMAC_LOOP_OFF |
  SK_GMAC_PAUSE_ON | SK_GMAC_RESET_CLEAR);

This `msk_reset' in OpenBSD is invoked in `msk_attach' and `msk_watchdog',
but interestingly not in `msk_init_yukon'. So the above lines not only seem
to have been extracted in OpenBSD to a separated function, but they are not
used inside `msk_init_yukon'.

Who is the NetBSD maintainer for if_msk.c? Is there any reason for this?
For Ryota, this is my previous mail, so you can better understand:
http://mail-index.netbsd.org/tech-kern/2017/10/11/msg022430.html

Rocky

Possible double entries in pcidevs

[Rocky Hotas]

Hi all!
The NetBSD src/sys/dev/pci/pcidevs rev. 1.1298 contains the following line
related to PCI device id 0x436a:

product MARVELL YUKON_C055  0x436a  Yukon 88EC055
(added with rev. 1.907, fixed typo with rev. 1.915)

The same file in the OpenBSD sourcetree, anyway, relates this PCI device id
to another product:

product MARVELL YUKON_8058  0x436a  Yukon 88E8058

Keeping both the lines could be undesirable and may generate conflicts?
Some observations:

- In NetBSD pcidevs, in the comments at the beginning of file, a complete
list "of PCI codes can be found at" http://www.pcidatabase.com/. From my
location and also with Google, this site is unreachable. The same
information is kept by http://pci-ids.ucw.cz/, which works. So, I would
suggest to update the beginning of pcidevs, with this new reference.
- According to pci-ids.ucw.cz, the company `Marvell Technology Group Ltd.'
(vendor id 0x11ab) has

0x436a  88E8058 PCI-E Gigabit Ethernet Controller

So, this would make the OpenBSD pcidevs the correct one. Does anyone know
more about it? Again, I would suggest to update pcidevs.
Bye!

Rocky

MCLGETI in if_msk.c

[Rocky Hotas]

Hi all!
I am working to adapt the OpenBSD if_msk.c driver to NetBSD. If needed,
these are my previous messages as a recap:

http://mail-index.netbsd.org/tech-kern/2017/10/11/msg022430.html
http://mail-index.netbsd.org/tech-kern/2017/10/11/msg022431.html

OpenBSD made a huge modification to the `msk_newbuf' function with
rev. 1.72:

http://cvsweb.openbsd.org/cgi-bin/cvsweb/src/sys/dev/pci/if_msk.c?rev=1.72&content-type=text/x-cvsweb-markup

(see the commit comments for the details). They use the MCLGETI macro
defined here:

http://bxr.su/OpenBSD/sys/sys/mbuf.h#315

In NetBSD this is a function, not a macro, and it is used only internally
by these two drivers:

src/sys/dev/ic/arn5008.c
src/sys/dev/ic/arn9003.c

Moreover,

src/sys/dev/pci/if_iwn.c

uses an improved version (maybe useful for the above files too in the
future) of the same function.
The OpenBSD macro refers instead to the function `m_clget' defined here:

http://bxr.su/OpenBSD/sys/kern/uipc_mbuf.c#348

It is different, as regards the use of the pointer `struct pool *pp',
the variable `caddr_t buf' and the flow itself of the code.
Is there a way to adapt it to NetBSD even so? Or can NetBSD use a new
function which could do the same?
Bye!

Rocky



P. S.
Member `arpcom.ac_if' in OpenBSD is `sk_ethercom.ec_if' for NetBSD in the
header of the function.

Re: MCLGETI in if_msk.c

[Rocky Hotas]

Hi Christos!

> Sent: Friday, October 27, 2017 at 1:58 PM
> From: "Christos Zoulas" 
> To: tech-kern@netbsd.org
> Subject: Re: MCLGETI in if_msk.c
>
[...]
>
> OpenBSD:
> m = MCLGETI(NULL, M_DONTWAIT, NULL, MCLBYTES);
> if (m == NULL)
>   goto fail;
> NetBSD:
> MGETHDR(m, M_DONTWAIT, MT_DATA);
> if (m == NULL)
> goto fail; 
> MCLGET(m, M_DONTWAIT);
> if (m->m_flags & M_EXT) == 0) {
>   m_freem(m);
> goto fail;
>   }

I didn't paste the whole OpenBSD code of rev. 1.72, but it is exactly:

MGETHDR(m, M_DONTWAIT, MT_DATA);
if (m == NULL)
return (ENOBUFS);

MCLGETI(m, M_DONTWAIT, &sc_if->arpcom.ac_if, sc_if->sk_pktlen);
if ((m->m_flags & M_EXT) == 0) {
m_freem(m);
return (ENOBUFS);
}

IIUC, I'll simply leave the MGETHDR part unchanged, and I'll replace the
MCLGETI call with the `MCLGET(m, M_DONTWAIT)' call you suggested.

> It is not such a big deal :-)

At a first glance, it seemed to the unexperienced me :D

> Or if you want to allocate a size other
> than MCLBYTES (I am not sure if that works, perhaps we need a different
> pool), use:
>   _MCLGET(m, mcl_cache, size, M_DONTWAIT) 
> 
> It is ~trivial to add the macro in  and since we are porting
> too many OpenBSD drivers, perhaps we should. But this should be discussed
> in tech-net.

It can be surely considered. As a side note, I suggest that also the MCLGETI
declared twice in `src/sys/dev/ic/arn5008.c' and `src/sys/dev/ic/arn9003.c',
and also in `src/sys/dev/pci/if_iwn.c' can be re-arranged.
Thank you,

Rocky

> 
> christos
> 
> 

Device probing and driver attach

[Rocky Hotas]

Hi all!
I hope this is appropriate to this list.
When the OS is booted, it still doesn't know the devices that will be
present in the current machine and it doesn't then know what drivers
should be used. After the bootstrap, the system autoconfiguration is
performed: according to autoconf(9), "Autoconfiguration is the process
of matching hardware devices with an appropriate device driver". Here
follow some questions:
- For each detected device, the probe function of *any* driver is called?
It seems a cumbersome procedure, but boot time is not that much long.
- In principle, if one built a custom kernel including *only* the drivers
needed by its current machine, would the boot time get significantly
reduced?
- When a BIOS does not perform this operation, is during the
autoconfiguration that device BARs are written by the OS?
- autoconf(9) specifies that "The autoconfiguration framework itself is
implemented within the file sys/kern/subr_autoconf.c". I am not that
skilled with it: which is, in that file, the routine that searches for
new devices and calls driver probe functions?
Thanks to anyone who can answer to even just one... or half :) of the
questions.

Rocky

pci_mapreg_map and BAR from `Bus space tutorial'

[Rocky Hotas]

Hello!
A very useful tutorial is available for newbies like me in

 
 

with a first introduction to kernel PCI drivers. I hope this is the
right ML to write to, despite the (possibly) elementary questions that
follow.

I created the driver according to all the instructions till slide 47,
but `dmesg' prints: `can't map the BAR'. Being it a test device
inside a Cobalt machine emulated by gxemul, I really am not able to
identify the problem.

The PCI device driver uses the file `src/sys/dev/pci/faareg.h' to define
the BAR position:

#ifndef FAAREG_H
#define FAAREG_H

#define FAA_MMREG_BAR   0x10
 
#endif

(I built the kernel this way, but am not sure about the position of
`#endif').

Then, the `attach' function in the driver main file
`src/sys/dev/pci/faa.c' acts this way:

static void
faa_attach(device_t parent, device_t self, void *aux)
{
struct faa_softc *sc = device_private(self);
const struct pci_attach_args *pa = aux;

sc->sc_dev = self;

pci_aprint_devinfo(pa, NULL);

if (pci_mapreg_map(pa, FAA_MMREG_BAR, PCI_MAPREG_TYPE_MEM, 0, 
&sc->sc_regt, &sc->sc_regh, &sc->sc_reg_pa, 0) != 0 ) {
aprint_error_dev(sc->sc_dev, "can't map the BAR\n");
return;
}

aprint_normal_dev(sc->sc_dev, "regs at 0x%08x\n", (uint32_t)
sc->sc_reg_pa);
}

IIUC from the slides and the manpages, `pci_mapreg_map' (PCI-specific
way of mapping a memory space) invokes `bus_space_map' (generic
bus_space way of mapping a memory space).

My doubts are:

- What does exactly do `pci_mapreg_map'? Does it map the physical address
  contained in FAA_MMREG_BAR to a kernel virtual address?

- FAA_MMREG_BAR may contain the beginning address of a space to be
  mapped. But how does `pci_mapreg_map' know the extension of such a map?

- According to pci(9), the ``physical address of the mapping is in''
  FAA_MMREG_BAR. But this assumes that someone previously wrote
  FAA_MMREG_BAR with the physical address assigned to that specific PCI
  device. As regards a NetBSD system, who knows that physical address
  and who is supposed to write it on this PCI device's BAR, before it
  can be used in `pci_mapreg_map'? That is: does the OS rely on the BIOS
  configuration, or does the OS configure the BARs by itself?

It's not always simple to gather some introductory information and I'm
sorry if my questions are trivial or somewhat naive. If anyone knows
about some documentation (not included in the final references of the
mentioned tutorial), I would check it out.
As well, if anyone knows another simple way to emulate a basic PCI
device for test/educational purposes on NetBSD, it's definitely welcome.
Thank you even just for having read.

Rocky

Re: pci_mapreg_map and BAR from `Bus space tutorial'

[Rocky Hotas]

Hello Martin,
sorry for the delay and thanks a lot for your answer.

On giu 02  7:03, Martin Husemann wrote:
 
> > - FAA_MMREG_BAR may contain the beginning address of a space to be
> >   mapped. But how does `pci_mapreg_map' know the extension of such a map?
> 
> It is just an offset (like a register number), the BAR contains all info
> needed (or should, if setup has worked correctly).

Oh, ok, so the BAR does not specify just an address: it also specifies
the extension of the memory space. I could not guess this.

> If you remove your driver you can list the details from userland with
> pcictl(8).
[...]
> Usually firmware (or early bootloaders) configure the BARs properly, but
> sometimes they don't, and then NetBSD needs to run fixup code (which is
> a kernel option and not included in most kernels). The output
> of pcictl would help here.

Ok! With or without the driver, the output of pcictl(8) is the same and
I attach it to this message. The device we are talking about is in bus
pci0, dev 12, fun 0, so I ran from a root prompt `pcictl pci0 dump -d 12'.

> Give this is mips I assume PCI_MAPREG_TYPE_MEM is correct, but for a new
> device you may be unsure, and sometimes variants of devices exist that
> either make the type IO or MEM. You can query that in your driver with
> pci_mapreg_info() - but for the concrete case the userland pcictl output is
> easier.

Consider that this is a custom PCI device, created by the author of the
tutorial as statement in the sourcecode of gxemul which defines the
`cobalt' machine.
The tutorial used PCI_MAPREG_TYPE_MEM. Looking into the output of pcictl(8),
also `I/O space accesses: on' is on, but I don't know if it is significant.

Bye!

Rocky
PCI configuration registers:
  Common header:
0x00: 0x0001fabc 0x0003 0x0b41 0x

Vendor ID: 0xfabc
Device ID: 0x0001
Command register: 0x0003
  I/O space accesses: on
  Memory space accesses: on
  Bus mastering: off
  Special cycles: off
  MWI transactions: off
  Palette snooping: off
  Parity error checking: off
  Address/data stepping: off
  System error (SERR): off
  Fast back-to-back transactions: off
  Interrupt disable: off
Status register: 0x
  Immediate Readiness: off
  Interrupt status: inactive
  Capability List support: off
  66 MHz capable: off
  User Definable Features (UDF) support: off
  Fast back-to-back capable: off
  Data parity error detected: off
  DEVSEL timing: fast (0x0)
  Slave signaled Target Abort: off
  Master received Target Abort: off
  Master received Master Abort: off
  Asserted System Error (SERR): off
  Parity error detected: off
Class Name: processor (0x0b)
Subclass Name: Co-processor (0x40)
Interface: 0x00
Revision ID: 0x01
BIST: 0x00
Header Type: 0x00 (0x00)
Latency Timer: 0x00
Cache Line Size: 0bytes (0x00)

  Type 0 ("normal" device) header:
0x10: 0x1011 0x 0x 0x
0x20: 0x 0x 0x 0x
0x30: 0x 0x 0x 0x

Base address register at 0x10
  type: 32-bit nonprefetchable memory
  base: 0x1011
Base address register at 0x14
  not implemented
Base address register at 0x18
  not implemented
Base address register at 0x1c
  not implemented
Base address register at 0x20
  not implemented
Base address register at 0x24
  not implemented
Cardbus CIS Pointer: 0x
Subsystem vendor ID: 0x
Subsystem ID: 0x
Expansion ROM Base Address Register: 0x
  base: 0x
  Expansion ROM Enable: off
  Validation Status: Validation not supported
  Validation Details: 0x0
Reserved @ 0x34: 0x
Reserved @ 0x38: 0x
Maximum Latency: 0x00
Minimum Grant: 0x00
Interrupt pin: 0x00 (none)
Interrupt line: 0x00

  Device-dependent header:
0x40: 0x 0x 0x 0x
0x50: 0x 0x 0x 0x
0x60: 0x 0x 0x 0x
0x70: 0x 0x 0x 0x
0x80: 0x 0x 0x 0x
0x90: 0x 0x 0x 0x
0xa0: 0x 0x 0x 0x
0xb0: 0x 0x 0x 0x
0xc0: 0x 0x 0x 0x
0xd0: 0x 0x 0x 0x
0xe0: 0x 0x 0x 0x
0xf0: 0x 0x 0x 0x

Re: pci_mapreg_map and BAR from `Bus space tutorial'

[Rocky Hotas]

Hi!

On giu 08 15:31, Mouse wrote:
> Guessing should not be relevant; how BARs work is part of the PCI
> definition.

Yes, of course. What I was meaning is that it seems counterintuitive
that a single 32 bit register can at the same time specify a 32 bit
physical memory address and a memory region extent.
In the example of the output of `pcictl' for my test device, this is
exactly the case: only 1 non-zero BAR,

Base address register at 0x10
  type: 32-bit nonprefetchable memory
  base: 0x1011

I am stuck in trying to interpret this value, and to use it both for an
address and a range.

> I forget details, but I think things like "I/O mapping
> versus memory mapping" are present in read-only bits at the LSB end of
> the BAR.  NetBSD has a bunch of #defines for them; look for PCI_MAPREG_
> in pcireg.h - or, at least, that's where they are on 1.4T, 5.2, and
> 8.0; while I don't have anything more recent at ready hand to check,
> I'd guess that something that stable is probably going to stay stable.

It is still as you said, IIUC: starting from line 453 of the current
revision 1.138.2.2,

#define PCI_MAPREG_TYPE_MEM 0x
#define PCI_MAPREG_TYPE_ROM 0x
#define PCI_MAPREG_TYPE_IO  0x0001
#define PCI_MAPREG_ROM_ENABLE   0x0001

> >> Give this is mips I assume PCI_MAPREG_TYPE_MEM is correct, but for a
> >> new device you may be unsure, and sometimes variants of devices
> >> exist that either make the type IO or MEM.  You can query that in
> >> your driver with pci_mapreg_info() - but for the concrete case the
> >> userland pcictl output is easier.
> 
> In this particular case, sure; I suspect the person who wrote the
> double-quoted text above was trying to give advice applicable beyond
> the case at immediate hand.

Got it!

> PCI is defined to have two address spaces, called I/O space and memory
> space.  On x86, these normally correspond to I/O instructions
> (inb/outb/etc) and memory-mapped I/O, respectively.  On other machines,
> this often works differently; some PCI attachments present PCI I/O
> space as memory-mapped too, just in a different part of the address
> space.

Yes, this may change according to the architecture. Here it's not x86.
If you remember some source, or documentation, I would check it out.

> The line you cite
> 
> > Command register: 0x0003
> >   I/O space accesses: on
> 
> is a PCI thing, indicating that the device is configured to respond to
> a PCI access marked as an I/O space access, provided it's mapped by a
> suitable mapping register.

This makes me wonder if the tutorial is correct and why.

The main fact here (at least, for a beginner) is that it's extremely
difficult to make multiple attempts, check and verify, as you would with
a normal C program (print something, do anything useful to let you know
how it works). Again, if you (or someone else) have any advice on how to
do this with kernel dmesg and an example PCI device, it would be very
useful.

Here, for example, I wonder what (in detail) makes pci_mapreg_map fail,
what kind of space actually needs the PCI test device: in other words,
some more information about this mapping.

> For a machine that already has a NetBSD port supporting PCI, those
> details should already be handled for you by the bus_space
> implementation for that port.  Your driver just needs to know "is this
> a (PCI) I/O space mapping or a (PCI) memory space mapping?", and
> sometimes not even that; what it turns into at the machine-code level
> is hidden by the bus_space layer.  See pci_mapreg_type(),
> pci_mapreg_map(), etc.

Maybe I can check the result of pci_mapreg_type(), to obtain more
information.

Anyone who can provide some other basic hints is very welcomed. In the
meanwhile, thanks a lot!

Rocky

Re: pci_mapreg_map and BAR from `Bus space tutorial'

[Rocky Hotas]

On giu 10 11:50, Martin Husemann wrote:

> so it is PCI_MAPREG_TYPE_MEM ("memory" in the type line), and the mapping
> should just work.

Ok! And I guess that invoking pci_mapreg_type() would give the same
result.

> Does the map address (0x1011) make sense on that machine?

I really don't know. The manual of MIPS R4000 is available at

 

but not the one of R5000, the processory family which should be used in the
emulated Cobalt machine:

 

Here,

 


(``an overview of the management of physical and virtual memory in the
MIPS R4x00, R5000, R8000, and R1 processors'')

the address 0x1011 belongs to a `user process virtual space, mapped
and cached', both when the processor uses 32 and 64 bit addresses. I
think however that the emulated Cobalt in gxemul uses 32 bit addresses.

Rocky

Re: pci_mapreg_map and BAR from `Bus space tutorial'

[Rocky Hotas]

On giu 10 12:54, Rocky Hotas wrote:

> the address 0x1011 belongs to a `user process virtual space, mapped
> and cached', both when the processor uses 32 and 64 bit addresses.

No, sorry, that was about Virtual addresses, not physical ones. Here,

 
<https://techpubs.jurassic.nl/manuals/hdwr/developer/DevDrvrO2_PG/sgi_html/ch15.html#id14958>

it is only stated that ``the relationship between the PCI bus address
space and the system memory physical address space differs from one
system type to another''.
So, basically it gives no information.

Rocky

Re: pci_mapreg_map and BAR from `Bus space tutorial'

[Rocky Hotas]

On giu 10  7:05, Mouse wrote:

> You can't.  If you have read-only access to the BAR - which is what
> pcictl gives you - then all you can get is the base address.  You need
> read/write access to get the range as well.  (In case you haven't yet
> seen the trick: you read the value and save it, then you write all-1s
> and read it back, to see which bits are read-only, then you write back
> the value you saved.  Yes, this implies that the size of the range is
> always a power of two.  As I said, clever, but I wish they'd kept their
> cleverness a bit more in check.)

Oh, got it. I checked some more documentation and tried the procedure
you mentioned. It works as expected: for example, my WiFi card ral0
requires a 2^16 bytes range, in its only active BAR. Great!

> I don't.  I have never worked with a cobalt and I don't know whether
> there is a de-facto standard for how MIPS interfaces to PCI.

NP. But if you know some source of documentation about x86 or some other
architectures, it is useful as well.

> Can't you print things?  I've done that often enough with kernel code.
> If -current has broken "printf from the driver" debugging, I'd call
> that a crippling regression.
[...]
> You could always do something like
> 
> (in your driver's .c file)
[...] 
> (in the implementation of pci_mapreg_map)
> 
> int mapreg_debug = 0;
>   ...
>   if (mapreg_debug) printf("...", ...);
> 
> It's a horrible thing to do for "permanent" (production) code, but I
> see nothing at all wrong with it for experimental debugging.

Thank you so much! I am not sure now to have figured it out correctly,
but I can try. Does this have any relation with setting pci_conf_debug
to 1 in `src/sys/dev/pci/pciconf.c', which was suggested in another
reply?

Rocky

Re: pci_mapreg_map and BAR from `Bus space tutorial'

[Rocky Hotas]

Hello Radoslaw,

On giu 11  1:02, Radoslaw Kujawa wrote:

> It seems that in 2018 cobalt port was switched to common MIPS bus space
> implementation. As a part of this implementation, it started to be more
> strict about verification of addresses used by device during bus_space_map.
[...] 
> Please try the attached gxemul patch.

Thank you so much for this message and the previous ones. Even if I do not
have the necessary knowledge, your comments gave several clarifications.

Also, thanks to Martin for mentioning you and for all his help.

Yes, the 2012 version of gxemul apparently can't compile due to those
warnings treated as errors. So, I tried to figure out what modifications
you made to it (not as many; they can be found through a simple recursive
grep) and applied them to the current gxemul version. This way, I could
use it.

I applied your patch and it works for me the same way. Thanks!

As a sidenote: I found your bus_space_tutorial very useful, the only one
available about this topic. For this problem, I suspended it at slide
47 of your pdf.
My intention is to follow it entirely, and then to elaborate and expend
it (adding comments, citations from the manpages), in order to keep a
useful and up-to-date introduction for a newbie. Would you agree?

I hope to be able to follow the remainder of the tutorial by my own. But
if some other problem could arise, can I contact you in future?
Obviously only if and when strictly necessary. 

In the meanwhile, thanks a lot for your time.

Bye!

Rocky


P.S.
I don't know if it's relevant, but something weird instead happens if I
try to write 0x in the faa BAR.
Before your patch, after the write, the BAR provided the value
0xFFF0: I was expecting 0xFFF0.
After your patch, and after writing 0x, BAR showed an even weirder
value: 0x000F. I used pcictl(8) for these operations. However, it all
seems to work from the dmesg, so maybe they are due to the fact that
this PCI device is not real.

Re: pci_mapreg_map and BAR from `Bus space tutorial'

[Rocky Hotas]

Hi :)!

> Sent: Sunday, June 14, 2020 at 7:21 PM
> From: "Radoslaw Kujawa" 
> To: "Rocky Hotas" 
> Cc: "Martin Husemann" , tech-kern@NetBSD.org
> Subject: Re: pci_mapreg_map and BAR from `Bus space tutorial'

> There's also a good general tutorial about NetBSD drivers, by Jochen
> Kunz, that is referenced at the end of bus_space tutorial.

Yes, I started reading it, too. As regards PCI, it seems sometimes too
wide and slightly off-topic, but it's also a basic and accurate source
of notions.

> The slides were not designed as a stand-alone learning material - I just
> used them to track the progress of a live tutorial, and emphasize
> important areas. So slides obviously lack any background information or
> comments.

I was sure this material was intended to be real-time commented. In fact
some EuroBSDCon video streamings are available on Youtube, but in the
2012 case I could not find your tutorial.
Just in case a video already exists and has been published somewhere, and
only if you want to share it, let me know. But don't worry otherwise.

> As you have probably noticed, the source to slides and examples is
> available on GitHub. However, if you are serious about expanding the
> tutorial please contact me off-list, as I am getting more and more
> hesitant to continue using GitHub for anything (I'll arrange a
> self-hosted repo).

Got it. It's now too early to discuss about this, but if I manage to
write something, I'll keep this in mind and inform you.

> Sure, feel free to contact me.
>
> Due to time constraints, I cannot guarantee a timely response, but I am
> very much interested in any further problems, or general comments that
> you may have.

Thank you so much, I'm glad to know this. No problem for any time-delay.

> I honeslty have no idea, debugging this would require digging into
> gxemul internals.

Ok! This is just a secondary issue now, but I mentioned it FTR.
Bye!

Rocky

Waiting for a bit in a register to be cleared: which strategy?

[Rocky Hotas]

Hello!
In order to read register A, I need to wait that a bit is cleared in
register B (which announces that the computation is over and the result
is available in register A). Both the registers are mapped in kernel
virtual address space through the bus_space(9) framework. This check is
inside a driver file.

As a temporary solution, I chose a brutal while with a brutal 32 bit
wise check:

#define NOT_CLEARED 0x001

while ((bus_space_read_4(mytag, myhandle, REG_A))
& (uint32_t) NOT_CLEARED) {
}

result = bus_space_read_4(mytag, myhandle, REG_B);

This works only because REG_A has all 0s except the LSB: when the LSB
becomes 0, too, the while exits. But I am actually interested only in
the LSB and would like to disregard the other 7 bits in the register.
What could it be the most efficient way to accomplish this?

When a bit must be set, there's the macro __BIT(n) in

 

Is there something similar for when just a single bit must be read?

This way, if for some reason the LSB in REG_A is never cleared (the
device is not working, or similar), the while never exits. Is it
available, inside the kernel, some function like sleep, or wait, so
that a maximum timeout can be set?

Summarising, my guess for the best solution in this case is: reading a
single bit with something similar to __BIT and set a maximum amount of
time for the while cycle. If better solutions are available, I'm ready
to follow any suggestion.

Bye!

Rocky

Re: Waiting for a bit in a register to be cleared: which strategy?

[Rocky Hotas]

On lug 03 14:56, Taylor R Campbell wrote:

> It's quite common to read a whole device register just to get at a
> single bit.  Don't worry about the efficiency -- the cost of the I/O
> transaction over the PCI bus or similar far exceeds the cost of
> pulling one bit out of 32.

OK then! Great!

> The usual idea for __BIT is that if there's a device register FOO, and
> it has various fields BAR, BAZ, and QUUX, with a hardware manual that
> says:
[...]
>   if ((foo & FOO_BAR) == 0)
>   return ENOENT;
>   baz = __SHIFTOUT(foo, FOO_BAZ);
>   quux = __SHIFTOUT(foo, FOO_QUUX);

Thanks for this very explanatory example.

> A typical approach is to set a reasonable timeout, either in register
> reads or in microseconds, and count down to it:
> 
>   unsigned timo = 1000;
> 
>   while ((bus_space_read_4(bst, bsh, FOO) & FOO_BAR) == 0) {
>   if (--timo == 0)
>   return ETIMEDOUT;
>   /* optionally, space the reads out by a microsecond */
>   DELAY(1);
>   }

OK! I think I would insert a delay.

> If you might need to wait for longer periods of time, like
> milliseconds, then you can use kpause with mstohz which lets other
> threads run, and if you're working under a lock, you can pass it to
> kpause to release the lock while other threads run.
> 
>   unsigned timo = 1000;
> 
>   mutex_enter(&sc->sc_lock);
>   while ((bus_space_read_4(bst, bsh, FOO) & FOO_BAR) == 0) {
>   if (--timo == 0)
>   return ETIMEDOUT;
>   kpause("foobar", false, mstohz(10), &sc->sc_lock);
>   }

This is also very useful. I never used a mutex, however, but:
- maybe I can use it just adding a sc_lock member in the driver's softc
  and use your example as a model;
- otherwise, I can completely avoid the lock, passing a NULL as last
  argument for `kpause'.

Hope this is correct in both cases.

> However, if you may need to wait for a long period of time, you should
> see if there's a way to get an interrupt notification instead of
> polling the device register, and use a condvar to signal the
> notification from the interrupt handler and to wait for the
> notification elsewhere in software.

In fact, I'm using this device:

 

and the bit of the question is the LSB of the status register 0x20,
which is cleared when the factorial computation is over. It is also
possible to raise an interrupt when this event occurs, but I do not know
interrupt handling at all in NetBSD kernel. If you know some simple
example, I would check them out. But so far I can't figure them out.

Thanks for your very useful suggestions!

Rocky

Re: Waiting for a bit in a register to be cleared: which strategy?

[Rocky Hotas]

On lug 03 11:15, Mouse wrote:

> I'm not sure what's "brutal" about those, but OK...

My ignorance about this kind of operations. Now I better understood,
thanks to your and Taylor's comments.

> Actually, because of your "& (uint32_t) NOT_CLEARED", it works
> regardless of the other bits in register A: you are masking off all but
> the low bit.

Oh, my fault. You are right.

> The other 31 bits - you're using bus_space_read_4, so it's reading 31
> bits.  If register A is an 8-bit register

No, in this case it is a 32 bit register: this is why I used
bus_space_read_4.

> Efficient in terms of what resource?  That is, what measure of
> efficiency are you interested in here?

Above all, the code complexity (not the hardware resource consumption).

> I'm having trouble thinking of one for which there's anything
> wrong with what you have.

OK, this is good!

> Check out the definition of __BIT - it is no different in practice
> from more or less what you're already doing.

Yes, now I better understood.

> There are multiple ways to do this.
> 
> The simplest is probably to just put a limit on the iteration count:
[...] 
> But this makes the timeout inherently dependent on the host CPU speed.

Yes, I excluded this because I would like to avoid using the CPU for
(potentially) millions of cycles and - as you said - because of the
CPU-dependent variable increment.

> If that's a problem, you could add a delay in the loop

I think this is the best in my case.

> If the device can be made to generate an interrupt when the bit
> changes, another answer is to make your driver enable that interrupt
> and go to sleep waiting for it.  The changes for that are too large for
> me to give here; depending on the surrounding code structure, they may
> involve significant rearrangement.

Yes, check out my answer to Taylor, with a link to the device
documentation. Interrupts are maybe the ideal solution, but I'm
completely unable to handle them in a driver. I'm a complete newbie.

> The last two options have the advantage that they release the CPU to do
> other useful work while waiting.  They have the disadvantage that they
> decrease responsiveness - the delay from the bit changing to the code
> reading register B increases significantly.

Yes, of coure.

> One is that busy-waiting is inefficient in that it does not allow the
> CPU to do any other useful work while waiting; if the delay is long
> compared to the time required to context-switch to doing other useful
> work, and there is, or might be, other useful work to do, this can be
> significant.

I think the bit is cleared almost immediately, according to how I guess
the device works, so this could not be the case.

> The other is that, as I wrote, for some buses and some devices, you
> have to access a register as the correct width or it will not work
[...]
> I don't
> know enough about your setup to tell whether it's an actual mismatch,
> nor, if it is, whether the difference matters.

I am sure that a 32 bit access is correct. My reply to Taylor (see the
bottom of the text) should provide all the elements to evaluate the
device.

Thanks for your help!

Rocky

Re: Waiting for a bit in a register to be cleared: which strategy?

[Rocky Hotas]

On lug 04  8:02, Iain Hibbert wrote:

> see pci_intr(9) and look at what a device in dev/pci does. I don't know 
> which is 'simple' but sv.c seems pretty simple in what it does with 
> interrupts the sv_intr() function is pretty small. You set up the 
> interrupt, enable it and your function gets called (in interrupt context). 

It was exactly what I was meaning: few lines, easy to read for someone
who has never seen an interrupt handling implementation.
Thank you, also for pci_intr(9)!

Rocky

Re: Waiting for a bit in a register to be cleared: which strategy?

[Rocky Hotas]

On lug 03 20:15, Mouse wrote:

> Then, yes, just loop.  First make it work, then make it better.

Ok! I did exactly this way.

> Well, be aware that DELAY() on many machines, especially for small
> arguments, _is_ just a cycle-burning loop - it just multiplies the
> argument by a constant calculated based on CPU speed.

Yes, IIUC from the manpages, DELAY(n) uses the argument n in a similar
way mstohz(9) would.

> > Yes, of coure.
> 
> If that's "of course" to you, you're not a _complete_ newbie. :-)

:) I tried to follow the logic reasoning behind your text. I have some
theory background, but reading code (and even more, writing it) is still
a big challenge.

> I'd say, just use the simple loop.  Once you have that much working,
> *then* worry about things like handling (pseudo-)hardware failures or
> interrupts, or sleeping instead of busy-waiting.

Ok, I'm relieved, because just did as you suggested. The loop works; I
honestly think that the bit is cleared in a negligible amount of time,
but it's right and proper to handle a failure. As a next improvement,
I'll try with waiting and then with an interrupt, if I manage to.
Thanks!

Rocky