Page MenuHomeFreeBSD

pciconf: Add option to write into a BAR region
Needs ReviewPublic

Authored by timo.voelker_fh-muenster.de on Wed, Mar 18, 1:34 PM.
Tags
None
Referenced Files
F149293698: D55915.diff
Mon, Mar 23, 1:21 PM
F149263847: D55915.id173849.diff
Mon, Mar 23, 8:19 AM
Unknown Object (File)
Sun, Mar 22, 8:34 AM
Unknown Object (File)
Thu, Mar 19, 1:03 PM
Unknown Object (File)
Wed, Mar 18, 2:03 PM
Subscribers

Details

Summary

Add option -Bw that allows to write into a BAR region.

Add an option -BD, which does the same as -D (dumps a BAR region). However, the B makes it clearer that it operates on a BAR region.

Test Plan

For example, a VirtIO PCI device

% pciconf -lb pci0:0:10:0
virtio_pci0@pci0:0:10:0:	class=0x020000 rev=0x00 hdr=0x00 vendor=0x1af4 device=0x1000 subvendor=0x1af4 subdevice=0x0001
    bar   [10] = type I/O Port, range 32, base 0, size 32, enabled
    bar   [18] = type Memory, range 32, base 0xe1852000, size 8192, enabled

The second 4 bytes of BAR 0x18 specifies the device's features based on the first 4 bytes.

% sudo pciconf -BD pci0:0:10:0 0x18 0x0 2 | hexdump -C
00000000  01 00 00 00 01 00 00 00                           |........|

By writing a 0 in the first 4 bytes the device changes the second 4 bytes.

% sudo pciconf -Bw pci0:0:10:0 0x18 0 0
% sudo pciconf -BD pci0:0:10:0 0x18 0x0 2 | hexdump -C
00000000  00 00 00 00 a3 dd 2f 00                           |....../.|

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

I think rather than trying to overload -B, it would be better to just use a different getopt() letter for this mode. I would suggest using -R for reading from a BAR but keep 'D' as an alias for backwards compatibility, and using -W for writing into a BAR.

In D55915#1279706, @jhb wrote:

I think rather than trying to overload -B, it would be better to just use a different getopt() letter for this mode. I would suggest using -R for reading from a BAR but keep 'D' as an alias for backwards compatibility, and using -W for writing into a BAR.

I like that.

You only write a single value into the specified offset, right?
This should be clearly explained in the man page addition.
Or consider adding a possibility of writing the series of values starting at the given offset, of the specified width for each value (IMO it is fine as is, but needs to be properly documented).

In D55915#1279706, @jhb wrote:

I think rather than trying to overload -B, it would be better to just use a different getopt() letter for this mode. I would suggest using -R for reading from a BAR but keep 'D' as an alias for backwards compatibility, and using -W for writing into a BAR.

I thought about that too but decided against it because:

  1. As a user, I probably would find it less clear that a capital letter operates on a BAR region instead of the PCI config space.
  2. Once I understand that, I would expect that -R (which would do the same as -D) reads from a BAR region as -r reads from the PCI config space. However, these modes a quite different.

After I saw that -b is already overloaded, I decided to use -B.

As a compromise, I could add a -W and leave the rest as it is (no -R). What do you think?

In D55915#1280245, @kib wrote:

You only write a single value into the specified offset, right?
This should be clearly explained in the man page addition.
Or consider adding a possibility of writing the series of values starting at the given offset, of the specified width for each value (IMO it is fine as is, but needs to be properly documented).

In my use case, I only had to write a single value. Do you know use cases where a series of values needs to be written to a BAR region?

I tried to make it similar to -w (write to config space). This is the man page part about -w:

The -w option writes the value into a configuration space register at byte offset addr of device selector.

Do you find this more clear?

If so, I could try to write the part of -Bw more like that.

If not, we should change that too.
BTW: Do you know what a selector is? It is not a pciconf argument as I would expect since it is underlined.

In D55915#1280245, @kib wrote:

You only write a single value into the specified offset, right?
This should be clearly explained in the man page addition.
Or consider adding a possibility of writing the series of values starting at the given offset, of the specified width for each value (IMO it is fine as is, but needs to be properly documented).

In my use case, I only had to write a single value. Do you know use cases where a series of values needs to be written to a BAR region?

I tried to make it similar to -w (write to config space). This is the man page part about -w:

The -w option writes the value into a configuration space register at byte offset addr of device selector.

Do you find this more clear?

Yes

If so, I could try to write the part of -Bw more like that.

If not, we should change that too.
BTW: Do you know what a selector is? It is not a pciconf argument as I would expect since it is underlined.

A selector is the device specification, basically 'pci:b:f:d' for pciconf.

In D55915#1281289, @kib wrote:
In D55915#1280245, @kib wrote:

You only write a single value into the specified offset, right?
This should be clearly explained in the man page addition.
Or consider adding a possibility of writing the series of values starting at the given offset, of the specified width for each value (IMO it is fine as is, but needs to be properly documented).

In my use case, I only had to write a single value. Do you know use cases where a series of values needs to be written to a BAR region?

I tried to make it similar to -w (write to config space). This is the man page part about -w:

The -w option writes the value into a configuration space register at byte offset addr of device selector.

Do you find this more clear?

Yes

OK. This is the synopsis:

pciconf -Bw [-b | -h | -x] device bar offset value

and this is the current text:

The -Bw option writes to the BAR specified by its offset in config space bar. The value is written at byte offset offset in the BAR region.

How about this?

The -Bw option writes the value into a BAR region of a PCI device at the given byte offset. It uses the BAR specified by its offset in the config space bar of the PCI device specified by the selector device.

If so, I could try to write the part of -Bw more like that.

If not, we should change that too.
BTW: Do you know what a selector is? It is not a pciconf argument as I would expect since it is underlined.

A selector is the device specification, basically 'pci:b:f:d' for pciconf.

OK, my question was misleading. I mean, why is selector underlined? It is not a pciconf argument. The name of the argument it refers to is device. Shouldn't end the sentence like that:

... of device selector device.
In D55915#1281289, @kib wrote:
In D55915#1280245, @kib wrote:

You only write a single value into the specified offset, right?
This should be clearly explained in the man page addition.
Or consider adding a possibility of writing the series of values starting at the given offset, of the specified width for each value (IMO it is fine as is, but needs to be properly documented).

In my use case, I only had to write a single value. Do you know use cases where a series of values needs to be written to a BAR region?

I tried to make it similar to -w (write to config space). This is the man page part about -w:

The -w option writes the value into a configuration space register at byte offset addr of device selector.

Do you find this more clear?

Yes

OK. This is the synopsis:

pciconf -Bw [-b | -h | -x] device bar offset value

and this is the current text:

The -Bw option writes to the BAR specified by its offset in config space bar. The value is written at byte offset offset in the BAR region.

How about this?

The -Bw option writes the value into a BAR region of a PCI device at the given byte offset. It uses the BAR specified by its offset in the config space bar of the PCI device specified by the selector device.

Ok

If so, I could try to write the part of -Bw more like that.

If not, we should change that too.
BTW: Do you know what a selector is? It is not a pciconf argument as I would expect since it is underlined.

A selector is the device specification, basically 'pci:b:f:d' for pciconf.

OK, my question was misleading. I mean, why is selector underlined? It is not a pciconf argument. The name of the argument it refers to is device. Shouldn't end the sentence like that:

... of device selector device.

I think this was some kind of typo. There are several instances of it in the man page. I suggest providing a preparational patch that would fix this first.