Re: [PATCH pciutils v1] pciutils: add new readpci utility

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Oct 10, 2022 at 06:27:41PM -0700, Jesse Brandeburg wrote:
> Add the new utility 'readpci' in order to allow users to read and write the
> register address space located in the BAR designator + offset.
> 
> The reason that this app is better than what is generally available on the
> internet (there are several) is that this app integrates with the libpci
> and further benefits from pciutils like arguments and device
> specifications.
> 
> help output:
> 
> $ sudo ./readpci -h
> ./readpci: invalid option -- 'h'
> Usage: ./readpci [options] [device]   (/usr/local/share/pci.ids.gz)
> 
> Options:
> -w <value>              Value to write to the address
> -W <value>              Value to write to the address (no read)
> -a <value>              Register address
> -b <value>              BAR to access other than BAR 0
> -m                      Access MSI-X BAR instead of BAR 0
> -D                      PCI debugging
> -q                      Quiet mode, no banner
> -v                      Enable more verbose output
> Device:
> -d [<vendor>]:[<device>]                        Show selected devices
> -s [[[[<domain>]:]<bus>]:][<slot>][.[<func>]]   Show devices in selected slots

I like this idea.  I often use rdwrmem, which is more general-purpose,
but it's a bit of a hassle that it's not commonly installed like
pciutils is, and you have to manually come up with the physical
address of a BAR.

The names:
  "setpci" -- read and write PCI config space
  "readpci" -- read and write PCI MMIO BARs
are slightly confusing since both support reads *and* writes, and
there's no clear config space vs BAR connection in the names.

Would it make any sense to integrate this into setpci, e.g., by
adding an address format for BAR MMIO offsets?

> basic usage to read a register:
> 
> $ sudo ./readpci -s 17:0.0 -a 0xb8000
> 17:00.0 (8086:1592) - Device 8086:1592
> 0xb8000 == 0x1

Possibly annotate with the BAR # and the complete physical address (to
correlate with lspci or dmesg output, especially when debugging via
email)?  Maybe also useful with reading MSI-X BAR.  Looks like maybe
it already does some of this with "v".

Possibly fill out the value to indicate the access width, e.g.,
0x00000001 in this case?

> Currently the utility only allows reading or writing one 32 bit address at
> a time. The utility must be run as root.

Does this mean the *address* is limited to 32 bits, or the access size
is 32 bits?

> +++ b/readpci.man

> +Access to read and write registers in PCI configuration space is restricted to root,

IIUC, readpci is for MMIO, not config space.  But I guess readpci
still needs data from config space to figure out *where* to read?  But
I assume that would normally come from sysfs, not config space
directly, so we can account for _TRA offsets.  I assume that info is
sysfs is also root-only, so it amounts to the same thing.  And
/dev/mem itself is also root-only.

I guess I would say either just "readpci can only be used by root" or
list the items actually required (sysfs config info and /dev/mem) in
case access to them requires different CAP_SYS_* things.

> +So,
> +.I readpci
> +isn't available to normal users.

> +.B -b [<value>]
> +Optional parameter, defaults to 0 if not specified. BAR number to access if
> +other than BAR0.

Maybe move the main point ("BAR number") first, mention the "optional"
part later?

> +on any bus, "0.3" selects third function of device 0 on all buses and ".4" shows only
> +the fourth function of each device.

Isn't "0.3" the *fourth* function?  0.0, 0.1, 0.2, 0.3?  Maybe reword
to avoid the question of whether we start with "zeroth" or "first",
e.g., "0.3 selects function 3 of device 0 on all buses"?

> +There might be some, but none known at this time. If you find one please
> +let the list know.

Does this man page say what "the list" refers to?

Bjorn

[1] https://cmp.felk.cvut.cz/~pisa/linux/rdwrmem.c



[Index of Archives]     [DMA Engine]     [Linux Coverity]     [Linux USB]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Greybus]

  Powered by Linux