On 12/11/2015 05:13 PM, Sean O. Stalley wrote:
Thanks for doing this. I tried it out & everything worked well for me.
Only a couple things:
The memory regions described by EA also show up before the EA Capability
and appear as virtual, ex:
Region 0: [virtual] Memory at 80000000 ...
We are just dumping out the config space. lspci doesn't really know
where the kernel gets the reported BAR values.
I don't think we want to suppress the dumping of the BARs, or the
kernel's interpretation thereof.
I don't think we want these regions to be flagged as virtual.
I think this happens because lspci can't read/write to the BAR region,
and so it assumes it's looking at a VF.
The rest of my gripes are minor syntax stuff. see inline.
Thanks again,
Sean
On Fri, Dec 11, 2015 at 02:27:25PM -0800, David Daney wrote:
From: David Daney <david.daney@xxxxxxxxxx>
[...]
--- a/ls-caps.c
+++ b/ls-caps.c
@@ -1254,6 +1254,145 @@ cap_sata_hba(struct device *d, int where, int cap)
printf(" BAR??%d\n", bar);
}
[...]
+static void cap_ea(struct device *d, int where, int cap)
+{
+ unsigned int entry;
+ int entry_base = where + 4;
+ unsigned int num_entries = BITS((unsigned int)cap, 0, 6);
+ u8 htype = get_conf_byte(d, PCI_HEADER_TYPE) & 0x7f;
+
+ printf("Enhanced Allocation: Num Entries=%u", num_entries);
CamelCase.
Those are the names from the specification, I would like to keep them in
some form. As noted below, it may be desirable to squash it to "NumEntries"
+ if (htype == PCI_HEADER_TYPE_BRIDGE) {
+ byte fixed_sub, fixed_sec;
+
+ entry_base += 4;
+ if (!config_fetch(d, where + 4, 2)) {
+ printf("\n");
+ return;
+ }
+ fixed_sec = get_conf_byte(d, where + 4);
+ fixed_sub = get_conf_byte(d, where + 4);
+ printf(", secondary=%d, subordinate=%d", fixed_sec, fixed_sub);
+ }
+ printf("\n");
+ if (verbose < 2)
+ return;
+
+ for (entry = 0; entry < num_entries; entry++) {
+ int max_offset_high_pos, has_base_high, has_max_offset_high;
+ u32 entry_header;
+ u32 base, max_offset;
+ unsigned int es, bei, pp, sp, e, w;
+
+ if (!config_fetch(d, entry_base, 4))
+ return;
+ entry_header = get_conf_long(d, entry_base);
+ es = BITS(entry_header, 0, 3);
+ bei = BITS(entry_header, 4, 4);
+ pp = BITS(entry_header, 8, 8);
+ sp = BITS(entry_header, 16, 8);
+ w = BITS(entry_header, 30, 1);
+ e = BITS(entry_header, 31, 1);
+ if (!config_fetch(d, entry_base + 4, es * 4))
+ return;
+ printf("\t\tEntry-%u: Enable%c Writable%c, Entry Size=%u\n", entry,
I would change this line to:
printf("\t\tEntry %u: Enable%c Writable%c, EntrySize=%u\n", ...);
We could do that. I would let others decide if squashing "Entry Size"
to "EntrySize" is desirable.
Changes: ^ ^
"-" after Entry to a " " (to match the "Region %i" used for BARs)
make EntrySize CamelCase (to match the other fields in lspci)
[...]
+ printf("\n");
+ printf("\t\t\t Primary Properties (PP): %s\n", cap_ea_property(pp));
+ printf("\t\t\t Secondary Properties (SP): %s\n", cap_ea_property(sp));
"PP" & "SP" are probably sufficient, I don't think we need to write the whole thing out.
Also, for the case where the Primary property is valid, can we hide the Secondary property if it is 0xFF?
My concern is that people may read the "entry unavailable for use" line as a warning and think something is wrong.
Maybe it would be better to make it say "Secondary Properties (SP): Not Present", or something similar.
That is a good point. I will try something like that.
+
+ base = get_conf_long(d, entry_base + 4);
+ has_base_high = ((base & 2) != 0);
+ base &= ~3;
+
+ max_offset = get_conf_long(d, entry_base + 8);
+ has_max_offset_high = ((max_offset & 2) != 0);
+ max_offset |= 3;
+ max_offset_high_pos = entry_base + 12;
+
+ printf("\t\t\t Base: ");
+ if (has_base_high) {
+ u32 base_high = get_conf_long(d, entry_base + 12);
+
+ printf("%x", base_high);
+ max_offset_high_pos += 4;
+ }
+ printf("%08x\n", base);
+
+ printf("\t\t\t Max Offset: ");
CamelCase.
Again, I am trying to match the wording of the specification. Although
in this case I think I got it wrong. It should probably be squashed to
"MaxOffset'
[...]
Thanks for reviewing it.
David Daney
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
