Page MenuHomeFreeBSD
Differential D9345

ifnet: move the new ifnet_event EVENTHANDLER_DECLARE to net/if_var.h
ClosedPublic
Actions

Authored by decui_microsoft.com on Jan 26 2017, 5:22 AM.
Tags
None
Referenced Files
Unknown Object (File)
Feb 3 2025, 7:01 PM
Unknown Object (File)
Feb 1 2025, 1:21 PM
Unknown Object (File)
Jan 28 2025, 10:22 PM
Unknown Object (File)
Jan 28 2025, 9:35 PM
Unknown Object (File)
Jan 28 2025, 2:04 AM
Unknown Object (File)
Jan 24 2025, 1:37 AM
Unknown Object (File)
Jan 24 2025, 1:23 AM
Unknown Object (File)
Jan 18 2025, 2:26 PM
View All Files
Subscribers
dab
freebsd-net-list

Details

Reviewers
kmacy
kib
howard0su_gmail.com
honzhan_microsoft.com
ae
cem
jhb
• hselasky
np
delphij
glebius
dab
sepherosa_gmail.com
gnn
royger
rwatson
Commits
rS312916: ifnet: move the new ifnet_event EVENTHANDLER_DECLARE to net/if_var.h
Summary

This is to fix
svn commit: r312687 (ifnet: introduce event handlers for ifup/ifdown events)

Thank glebius for pointing this out:
"The network stuff shall not be added to sys/eventhandler.h"

Diff Detail

Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

decui_microsoft.com updated this revision to Diff 24465.Jan 26 2017, 5:22 AM
decui_microsoft.com retitled this revision from to ifnet: move the new ifnet_event EVENTHANDLER_DECLAREs to net/if_var.h.
decui_microsoft.com updated this object.
decui_microsoft.com edited the test plan for this revision. (Show Details)
decui_microsoft.com added reviewers: • hselasky, sepherosa_gmail.com, cem, np, kmacy, kib, honzhan_microsoft.com, howard0su_gmail.com, jhb, ae, delphij, royger, glebius, gnn, rwatson.
decui_microsoft.com added a subscriber: freebsd-net-list.
decui_microsoft.com retitled this revision from ifnet: move the new ifnet_event EVENTHANDLER_DECLAREs to net/if_var.h to ifnet: move the new ifnet_event EVENTHANDLER_DECLARE to net/if_var.h.Jan 26 2017, 5:23 AM
dab added a subscriber: dab.Jan 26 2017, 1:48 PM

Just a couple style nits.

sys/net/if_var.h
407

I would stick with the previous "ifup/ifdown" or go with "Interface up/down". I think "Interface up/ifdown" reads poorly.

408

It would be nice to have the 0/1 values for these two defines line up. In any case, it definitely seems like there is extraneous whitespace between "IFNET_EVENT_UP" and "0".

decui_microsoft.com added inline comments.Jan 26 2017, 2:14 PM
sys/net/if_var.h
407

I was trying to keep the consistency with Line 392, 395, 401 and 404.
I would tend to leave the patch as it is, if you won't strongly object to it. :-)

408

Actually the 2 lines do line up.
The visual confusion is due to the nasty Tab issue in the case of a patch. :-)

With "set list" in my Vim, the lines of the file net/if_var.h (rather than the patch with a leading + at the beginning of the lines) shows

#define IFNET_EVENT_UP^I^I0$
#define IFNET_EVENT_DOWN^I1$

^I means a Tab.
$ means a CR-LF.

dab accepted this revision.Jan 26 2017, 2:31 PM
dab added a reviewer: dab.
dab added inline comments.
sys/net/if_var.h
407

No objection to "Interface ..." but then I'd use "Interface up/down event" (the "if" in "ifdown" meaning "interface" is redundant).

But, then, this is a nit. I don't strongly object to the patch as-is.

408

Hmmm, you are right; Phabricator is deceiving me. They don't line up at all in the diff display it shows me, but you are right that they do in code. Sorry about that.

(Just a note that style(9) says to use just a single tab between name and value, which would mean they wouldn't line up. See similar situation at lines 185-186.)

This revision is now accepted and ready to land.Jan 26 2017, 2:31 PM
decui_microsoft.com added inline comments.Jan 26 2017, 3:59 PM
sys/net/if_var.h
407

Oh... Sorry, my bad! I failed to understand what you meant -- it's late at night here and obviously my eyes or brain didn't work very good :-)

decui_microsoft.com updated this revision to Diff 24479.Jan 26 2017, 4:04 PM
decui_microsoft.com edited edge metadata.

fixed the comment typo pointed out by David.

This revision now requires review to proceed.Jan 26 2017, 4:04 PM
glebius accepted this revision.Jan 26 2017, 9:57 PM
glebius edited edge metadata.
This revision is now accepted and ready to land.Jan 26 2017, 9:57 PM
sepherosa_gmail.com accepted this revision.Jan 27 2017, 2:43 AM
sepherosa_gmail.com edited edge metadata.
dab accepted this revision.Jan 27 2017, 2:26 PM
dab edited edge metadata.
Closed by commit rS312916: ifnet: move the new ifnet_event EVENTHANDLER_DECLARE to net/if_var.h (authored by dexuan). · Explain WhyJan 28 2017, 7:27 AM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathSize
sys/
net/
1 line
5 lines
sys/
7 lines

Diff 24479

View Options

sys/net/if.c

Show First 20 Lines • Show All 63 Lines • ▼ Show 20 Lines
#define PCIE_REG_MASK 0xFFF #define PCIE_REG_MASK 0xFFF
#define PCIE_ADDR_OFFSET(bus, slot, func, reg) \ #define PCIE_ADDR_OFFSET(bus, slot, func, reg) \
((((bus) & PCIE_BUS_MASK) << PCIE_BUS_SHIFT) | \ ((((bus) & PCIE_BUS_MASK) << PCIE_BUS_SHIFT) | \
(((slot) & PCIE_SLOT_MASK) << PCIE_SLOT_SHIFT) | \ (((slot) & PCIE_SLOT_MASK) << PCIE_SLOT_SHIFT) | \
(((func) & PCIE_FUNC_MASK) << PCIE_FUNC_SHIFT) | \ (((func) & PCIE_FUNC_MASK) << PCIE_FUNC_SHIFT) | \
((reg) & PCIE_REG_MASK)) ((reg) & PCIE_REG_MASK))
typedef void (*pci_host_generic_quirk_function)(device_t);
struct pci_host_generic_quirk_entry {
int impl;
int part;
int var;
int rev;
pci_host_generic_quirk_function func;
};
struct pci_host_generic_block_entry {
int impl;
int part;
int var;
int rev;
int bus;
int slot;
};
/* Forward prototypes */ /* Forward prototypes */
static uint32_t generic_pcie_read_config(device_t dev, u_int bus, u_int slot, static uint32_t generic_pcie_read_config(device_t dev, u_int bus, u_int slot,
u_int func, u_int reg, int bytes); u_int func, u_int reg, int bytes);
static void generic_pcie_write_config(device_t dev, u_int bus, u_int slot, static void generic_pcie_write_config(device_t dev, u_int bus, u_int slot,
u_int func, u_int reg, uint32_t val, int bytes); u_int func, u_int reg, uint32_t val, int bytes);
static int generic_pcie_maxslots(device_t dev); static int generic_pcie_maxslots(device_t dev);
static int generic_pcie_read_ivar(device_t dev, device_t child, int index, static int generic_pcie_read_ivar(device_t dev, device_t child, int index,
uintptr_t *result); uintptr_t *result);
static int generic_pcie_write_ivar(device_t dev, device_t child, int index, static int generic_pcie_write_ivar(device_t dev, device_t child, int index,
uintptr_t value); uintptr_t value);
static void pci_host_generic_apply_quirks(device_t);
static void thunderx2_ahci_bar_quirk(device_t);
struct pci_host_generic_quirk_entry pci_host_generic_quirks[] =
{
{CPU_IMPL_CAVIUM, CPU_PART_THUNDERX2, 0, 0, thunderx2_ahci_bar_quirk},
{0, 0, 0, 0, NULL}
};
struct pci_host_generic_block_entry pci_host_generic_blocked[] =
{
/* ThunderX2 AHCI on second socket */
{CPU_IMPL_CAVIUM, CPU_PART_THUNDERX2, 0, 0, 0x80, 0x10},
{0, 0, 0, 0, 0, 0}
};
andrewUnsubmitted
Not Done
Inline Actions

Why is this needed?

andrew: Why is this needed?
pdk_semihalf.comAuthorUnsubmitted
Not Done
Inline Actions

Second socket have SATA controller on chip. It is not exposed on board, and have bad adresses (like SATA on first socket). Instead of fixing addresses we block it.

pdk_semihalf.com: Second socket have SATA controller on chip. It is not exposed on board, and have bad adresses…
int int
pci_host_generic_core_attach(device_t dev) pci_host_generic_core_attach(device_t dev)
{ {
struct generic_pcie_core_softc *sc; struct generic_pcie_core_softc *sc;
int error; int error;
int rid; int rid;
sc = device_get_softc(dev); sc = device_get_softc(dev);
Show All 37 Linespci_host_generic_core_attach(device_t dev)
} }
error = rman_init(&sc->io_rman); error = rman_init(&sc->io_rman);
if (error) { if (error) {
device_printf(dev, "rman_init() failed. error = %d\n", error); device_printf(dev, "rman_init() failed. error = %d\n", error);
return (error); return (error);
} }
pci_host_generic_apply_quirks(dev);
return (0); return (0);
} }
static void
pci_host_generic_apply_quirks(device_t dev)
{
struct pci_host_generic_quirk_entry *quirk;
quirk = pci_host_generic_quirks;
while (1) {
if (quirk->impl == 0)
break;
if (CPU_MATCH(CPU_IMPL_MASK | CPU_PART_MASK,
quirk->impl, quirk->part, quirk->var, quirk->rev) &&
quirk->func != NULL)
quirk->func(dev);
quirk++;
}
}
static uint32_t static uint32_t
generic_pcie_read_config(device_t dev, u_int bus, u_int slot, generic_pcie_read_config(device_t dev, u_int bus, u_int slot,
u_int func, u_int reg, int bytes) u_int func, u_int reg, int bytes)
{ {
struct generic_pcie_core_softc *sc; struct generic_pcie_core_softc *sc;
bus_space_handle_t h; bus_space_handle_t h;
bus_space_tag_t t; bus_space_tag_t t;
uint64_t offset; uint64_t offset;
uint32_t data; uint32_t data;
struct pci_host_generic_block_entry *block;
if ((bus > PCI_BUSMAX) || (slot > PCI_SLOTMAX) || if ((bus > PCI_BUSMAX) || (slot > PCI_SLOTMAX) ||
(func > PCI_FUNCMAX) || (reg > PCIE_REGMAX)) (func > PCI_FUNCMAX) || (reg > PCIE_REGMAX))
return (~0U); return (~0U);
block = pci_host_generic_blocked;
while (1) {
if (block->impl == 0)
break;
if (CPU_MATCH(CPU_IMPL_MASK | CPU_PART_MASK,
block->impl, block->part, block->var, block->rev) &&
block->bus == bus && block->slot == slot)
return (~0);
block++;
}
sc = device_get_softc(dev); sc = device_get_softc(dev);
offset = PCIE_ADDR_OFFSET(bus, slot, func, reg); offset = PCIE_ADDR_OFFSET(bus, slot, func, reg);
t = sc->bst; t = sc->bst;
h = sc->bsh; h = sc->bsh;
switch (bytes) { switch (bytes) {
case 1: case 1:
▲ Show 20 Lines • Show All 224 Lines • ▼ Show 20 Linesstatic device_method_t generic_pcie_methods[] = {
DEVMETHOD(pcib_read_config, generic_pcie_read_config), DEVMETHOD(pcib_read_config, generic_pcie_read_config),
DEVMETHOD(pcib_write_config, generic_pcie_write_config), DEVMETHOD(pcib_write_config, generic_pcie_write_config),
DEVMETHOD_END DEVMETHOD_END
}; };
DEFINE_CLASS_0(pcib, generic_pcie_core_driver, DEFINE_CLASS_0(pcib, generic_pcie_core_driver,
generic_pcie_methods, sizeof(struct generic_pcie_core_softc)); generic_pcie_methods, sizeof(struct generic_pcie_core_softc));
static void thunderx2_ahci_bar_quirk(device_t dev)
{
/*
* XXX:
* On ThunderX2, AHCI BAR2 address is wrong. It needs to precisely
* match the one described in datasheet. Fixup it unconditionally.
andrewUnsubmitted
Not Done
Inline Actions

Is there an erratum for this?

andrew: Is there an erratum for this?
pdk_semihalf.comAuthorUnsubmitted
Not Done
Inline Actions

There is no erratum. Without this quirk kernel is trying to access bad address.

pdk_semihalf.com: There is no erratum. Without this quirk kernel is trying to access bad address.
andrewUnsubmitted
Not Done
Inline Actions

Why is the kernel trying to access a bad address?

andrew: Why is the kernel trying to access a bad address?
pdk_semihalf.comAuthorUnsubmitted
Not Done
Inline Actions

Kernel is trying to attach ahci. Look at log (without quirk of course) below it contains aforementioned bad address:

ahci0: <AHCI SATA controller> mem 0x41d4100041d30000-0x41d4100041d3ffff,0x41d20000-0x41d2ffff at device 16.0 on pci0                
PDK: virtual=0xffff000165388000 size=65536 pa=41d20000                                                                              
PDK: virtual=0xffff000165398000 size=65536 pa=41d4100041d30000                                                                      
ahci0: PDK: msix_table_bar: 24                                                                                                      
ahci0: PDK: msix_table_res: 0xfffffd0001e25180                                                                                      
ahci0: attempting to allocate 4 MSI-X vectors (4 supported)                                                                         
ahci0: using IRQs 11-14 for MSI-X                                                                                                                                                                                                                                                                                               
panic: vm_fault_hold: fault on nofault entry, addr: 0xffff000165398000
pdk_semihalf.com: Kernel is trying to attach ahci. Look at log (without quirk of course) below it contains…
*/
if (device_get_unit(dev) == 0) {
device_printf(dev, "running AHCI BAR fixup\n");
PCIB_WRITE_CONFIG(dev, 0, 16, 0, 0x18, 0x01440000, 4);
PCIB_WRITE_CONFIG(dev, 0, 16, 0, 0x1c, 0x40, 4);
PCIB_WRITE_CONFIG(dev, 0, 16, 1, 0x18, 0x01450000, 4);
PCIB_WRITE_CONFIG(dev, 0, 16, 1, 0x1c, 0x40, 4);
}
}
andrewUnsubmitted
Not Done
Inline Actions

Shouldn't you be getting this from the ACPI mcfg table?

andrew: Shouldn't you be getting this from the ACPI mcfg table?
pdk_semihalf.comAuthorUnsubmitted
Not Done
Inline Actions

Yes, I should get this from MCFG, but it doesn't provide information about pci on second socket. Furthermore segment passed to acpi_enable_pcie is 0 in both cases.

pdk_semihalf.com: Yes, I should get this from MCFG, but it doesn't provide information about pci on second socket.
andrewUnsubmitted
Not Done
Inline Actions

Will this always be unit 16, or would it be better to look at other information to detect when it's an ecam device.

andrew: Will this always be unit 16, or would it be better to look at other information to detect when…
pdk_semihalf.comAuthorUnsubmitted
Not Done
Inline Actions

It will be always unit 16. I've checked it once again and I have an idea. I will use _BBN to get ecam number. I'll update patches today so stay tuned.

pdk_semihalf.com: It will be always unit 16. I've checked it once again and I have an idea. I will use _BBN to…
View Options

sys/net/if_var.h

Show First 20 Lines • Show All 63 Lines • ▼ Show 20 Lines
#define PCIE_REG_MASK 0xFFF #define PCIE_REG_MASK 0xFFF
#define PCIE_ADDR_OFFSET(bus, slot, func, reg) \ #define PCIE_ADDR_OFFSET(bus, slot, func, reg) \
((((bus) & PCIE_BUS_MASK) << PCIE_BUS_SHIFT) | \ ((((bus) & PCIE_BUS_MASK) << PCIE_BUS_SHIFT) | \
(((slot) & PCIE_SLOT_MASK) << PCIE_SLOT_SHIFT) | \ (((slot) & PCIE_SLOT_MASK) << PCIE_SLOT_SHIFT) | \
(((func) & PCIE_FUNC_MASK) << PCIE_FUNC_SHIFT) | \ (((func) & PCIE_FUNC_MASK) << PCIE_FUNC_SHIFT) | \
((reg) & PCIE_REG_MASK)) ((reg) & PCIE_REG_MASK))
typedef void (*pci_host_generic_quirk_function)(device_t);
struct pci_host_generic_quirk_entry {
int impl;
int part;
int var;
int rev;
pci_host_generic_quirk_function func;
};
struct pci_host_generic_block_entry {
int impl;
int part;
int var;
int rev;
int bus;
int slot;
};
/* Forward prototypes */ /* Forward prototypes */
static uint32_t generic_pcie_read_config(device_t dev, u_int bus, u_int slot, static uint32_t generic_pcie_read_config(device_t dev, u_int bus, u_int slot,
u_int func, u_int reg, int bytes); u_int func, u_int reg, int bytes);
static void generic_pcie_write_config(device_t dev, u_int bus, u_int slot, static void generic_pcie_write_config(device_t dev, u_int bus, u_int slot,
u_int func, u_int reg, uint32_t val, int bytes); u_int func, u_int reg, uint32_t val, int bytes);
static int generic_pcie_maxslots(device_t dev); static int generic_pcie_maxslots(device_t dev);
static int generic_pcie_read_ivar(device_t dev, device_t child, int index, static int generic_pcie_read_ivar(device_t dev, device_t child, int index,
uintptr_t *result); uintptr_t *result);
static int generic_pcie_write_ivar(device_t dev, device_t child, int index, static int generic_pcie_write_ivar(device_t dev, device_t child, int index,
uintptr_t value); uintptr_t value);
static void pci_host_generic_apply_quirks(device_t);
static void thunderx2_ahci_bar_quirk(device_t);
struct pci_host_generic_quirk_entry pci_host_generic_quirks[] =
{
{CPU_IMPL_CAVIUM, CPU_PART_THUNDERX2, 0, 0, thunderx2_ahci_bar_quirk},
{0, 0, 0, 0, NULL}
};
struct pci_host_generic_block_entry pci_host_generic_blocked[] =
{
/* ThunderX2 AHCI on second socket */
{CPU_IMPL_CAVIUM, CPU_PART_THUNDERX2, 0, 0, 0x80, 0x10},
{0, 0, 0, 0, 0, 0}
};
andrewUnsubmitted
Not Done
Inline Actions

Why is this needed?

andrew: Why is this needed?
pdk_semihalf.comAuthorUnsubmitted
Not Done
Inline Actions

Second socket have SATA controller on chip. It is not exposed on board, and have bad adresses (like SATA on first socket). Instead of fixing addresses we block it.

pdk_semihalf.com: Second socket have SATA controller on chip. It is not exposed on board, and have bad adresses…
int int
pci_host_generic_core_attach(device_t dev) pci_host_generic_core_attach(device_t dev)
{ {
struct generic_pcie_core_softc *sc; struct generic_pcie_core_softc *sc;
int error; int error;
int rid; int rid;
sc = device_get_softc(dev); sc = device_get_softc(dev);
Show All 37 Linespci_host_generic_core_attach(device_t dev)
} }
error = rman_init(&sc->io_rman); error = rman_init(&sc->io_rman);
if (error) { if (error) {
device_printf(dev, "rman_init() failed. error = %d\n", error); device_printf(dev, "rman_init() failed. error = %d\n", error);
return (error); return (error);
} }
pci_host_generic_apply_quirks(dev);
return (0); return (0);
} }
static void
pci_host_generic_apply_quirks(device_t dev)
{
struct pci_host_generic_quirk_entry *quirk;
quirk = pci_host_generic_quirks;
while (1) {
if (quirk->impl == 0)
break;
if (CPU_MATCH(CPU_IMPL_MASK | CPU_PART_MASK,
quirk->impl, quirk->part, quirk->var, quirk->rev) &&
quirk->func != NULL)
quirk->func(dev);
quirk++;
}
}
static uint32_t static uint32_t
generic_pcie_read_config(device_t dev, u_int bus, u_int slot, generic_pcie_read_config(device_t dev, u_int bus, u_int slot,
u_int func, u_int reg, int bytes) u_int func, u_int reg, int bytes)
{ {
struct generic_pcie_core_softc *sc; struct generic_pcie_core_softc *sc;
bus_space_handle_t h; bus_space_handle_t h;
bus_space_tag_t t; bus_space_tag_t t;
uint64_t offset; uint64_t offset;
uint32_t data; uint32_t data;
struct pci_host_generic_block_entry *block;
if ((bus > PCI_BUSMAX) || (slot > PCI_SLOTMAX) || if ((bus > PCI_BUSMAX) || (slot > PCI_SLOTMAX) ||
(func > PCI_FUNCMAX) || (reg > PCIE_REGMAX)) (func > PCI_FUNCMAX) || (reg > PCIE_REGMAX))
return (~0U); return (~0U);
block = pci_host_generic_blocked;
while (1) {
if (block->impl == 0)
break;
if (CPU_MATCH(CPU_IMPL_MASK | CPU_PART_MASK,
block->impl, block->part, block->var, block->rev) &&
block->bus == bus && block->slot == slot)
return (~0);
block++;
}
sc = device_get_softc(dev); sc = device_get_softc(dev);
offset = PCIE_ADDR_OFFSET(bus, slot, func, reg); offset = PCIE_ADDR_OFFSET(bus, slot, func, reg);
t = sc->bst; t = sc->bst;
h = sc->bsh; h = sc->bsh;
switch (bytes) { switch (bytes) {
case 1: case 1:
▲ Show 20 Lines • Show All 224 Lines • ▼ Show 20 Linesstatic device_method_t generic_pcie_methods[] = {
DEVMETHOD(pcib_read_config, generic_pcie_read_config), DEVMETHOD(pcib_read_config, generic_pcie_read_config),
DEVMETHOD(pcib_write_config, generic_pcie_write_config), DEVMETHOD(pcib_write_config, generic_pcie_write_config),
DEVMETHOD_END DEVMETHOD_END
}; };
DEFINE_CLASS_0(pcib, generic_pcie_core_driver, DEFINE_CLASS_0(pcib, generic_pcie_core_driver,
generic_pcie_methods, sizeof(struct generic_pcie_core_softc)); generic_pcie_methods, sizeof(struct generic_pcie_core_softc));
static void thunderx2_ahci_bar_quirk(device_t dev)
{
/*
* XXX:
* On ThunderX2, AHCI BAR2 address is wrong. It needs to precisely
* match the one described in datasheet. Fixup it unconditionally.
andrewUnsubmitted
Not Done
Inline Actions

Is there an erratum for this?

andrew: Is there an erratum for this?
pdk_semihalf.comAuthorUnsubmitted
Not Done
Inline Actions

There is no erratum. Without this quirk kernel is trying to access bad address.

pdk_semihalf.com: There is no erratum. Without this quirk kernel is trying to access bad address.
andrewUnsubmitted
Not Done
Inline Actions

Why is the kernel trying to access a bad address?

andrew: Why is the kernel trying to access a bad address?
pdk_semihalf.comAuthorUnsubmitted
Not Done
Inline Actions

Kernel is trying to attach ahci. Look at log (without quirk of course) below it contains aforementioned bad address:

ahci0: <AHCI SATA controller> mem 0x41d4100041d30000-0x41d4100041d3ffff,0x41d20000-0x41d2ffff at device 16.0 on pci0                
PDK: virtual=0xffff000165388000 size=65536 pa=41d20000                                                                              
PDK: virtual=0xffff000165398000 size=65536 pa=41d4100041d30000                                                                      
ahci0: PDK: msix_table_bar: 24                                                                                                      
ahci0: PDK: msix_table_res: 0xfffffd0001e25180                                                                                      
ahci0: attempting to allocate 4 MSI-X vectors (4 supported)                                                                         
ahci0: using IRQs 11-14 for MSI-X                                                                                                                                                                                                                                                                                               
panic: vm_fault_hold: fault on nofault entry, addr: 0xffff000165398000
pdk_semihalf.com: Kernel is trying to attach ahci. Look at log (without quirk of course) below it contains…
*/
if (device_get_unit(dev) == 0) {
device_printf(dev, "running AHCI BAR fixup\n");
PCIB_WRITE_CONFIG(dev, 0, 16, 0, 0x18, 0x01440000, 4);
PCIB_WRITE_CONFIG(dev, 0, 16, 0, 0x1c, 0x40, 4);
PCIB_WRITE_CONFIG(dev, 0, 16, 1, 0x18, 0x01450000, 4);
PCIB_WRITE_CONFIG(dev, 0, 16, 1, 0x1c, 0x40, 4);
}
}
andrewUnsubmitted
Not Done
Inline Actions

Shouldn't you be getting this from the ACPI mcfg table?

andrew: Shouldn't you be getting this from the ACPI mcfg table?
pdk_semihalf.comAuthorUnsubmitted
Not Done
Inline Actions

Yes, I should get this from MCFG, but it doesn't provide information about pci on second socket. Furthermore segment passed to acpi_enable_pcie is 0 in both cases.

pdk_semihalf.com: Yes, I should get this from MCFG, but it doesn't provide information about pci on second socket.
andrewUnsubmitted
Not Done
Inline Actions

Will this always be unit 16, or would it be better to look at other information to detect when it's an ecam device.

andrew: Will this always be unit 16, or would it be better to look at other information to detect when…
pdk_semihalf.comAuthorUnsubmitted
Not Done
Inline Actions

It will be always unit 16. I've checked it once again and I have an idea. I will use _BBN to get ecam number. I'll update patches today so stay tuned.

pdk_semihalf.com: It will be always unit 16. I've checked it once again and I have an idea. I will use _BBN to…
View Options

sys/sys/eventhandler.h

Show First 20 Lines • Show All 63 Lines • ▼ Show 20 Lines
#define PCIE_REG_MASK 0xFFF #define PCIE_REG_MASK 0xFFF
#define PCIE_ADDR_OFFSET(bus, slot, func, reg) \ #define PCIE_ADDR_OFFSET(bus, slot, func, reg) \
((((bus) & PCIE_BUS_MASK) << PCIE_BUS_SHIFT) | \ ((((bus) & PCIE_BUS_MASK) << PCIE_BUS_SHIFT) | \
(((slot) & PCIE_SLOT_MASK) << PCIE_SLOT_SHIFT) | \ (((slot) & PCIE_SLOT_MASK) << PCIE_SLOT_SHIFT) | \
(((func) & PCIE_FUNC_MASK) << PCIE_FUNC_SHIFT) | \ (((func) & PCIE_FUNC_MASK) << PCIE_FUNC_SHIFT) | \
((reg) & PCIE_REG_MASK)) ((reg) & PCIE_REG_MASK))
typedef void (*pci_host_generic_quirk_function)(device_t);
struct pci_host_generic_quirk_entry {
int impl;
int part;
int var;
int rev;
pci_host_generic_quirk_function func;
};
struct pci_host_generic_block_entry {
int impl;
int part;
int var;
int rev;
int bus;
int slot;
};
/* Forward prototypes */ /* Forward prototypes */
static uint32_t generic_pcie_read_config(device_t dev, u_int bus, u_int slot, static uint32_t generic_pcie_read_config(device_t dev, u_int bus, u_int slot,
u_int func, u_int reg, int bytes); u_int func, u_int reg, int bytes);
static void generic_pcie_write_config(device_t dev, u_int bus, u_int slot, static void generic_pcie_write_config(device_t dev, u_int bus, u_int slot,
u_int func, u_int reg, uint32_t val, int bytes); u_int func, u_int reg, uint32_t val, int bytes);
static int generic_pcie_maxslots(device_t dev); static int generic_pcie_maxslots(device_t dev);
static int generic_pcie_read_ivar(device_t dev, device_t child, int index, static int generic_pcie_read_ivar(device_t dev, device_t child, int index,
uintptr_t *result); uintptr_t *result);
static int generic_pcie_write_ivar(device_t dev, device_t child, int index, static int generic_pcie_write_ivar(device_t dev, device_t child, int index,
uintptr_t value); uintptr_t value);
static void pci_host_generic_apply_quirks(device_t);
static void thunderx2_ahci_bar_quirk(device_t);
struct pci_host_generic_quirk_entry pci_host_generic_quirks[] =
{
{CPU_IMPL_CAVIUM, CPU_PART_THUNDERX2, 0, 0, thunderx2_ahci_bar_quirk},
{0, 0, 0, 0, NULL}
};
struct pci_host_generic_block_entry pci_host_generic_blocked[] =
{
/* ThunderX2 AHCI on second socket */
{CPU_IMPL_CAVIUM, CPU_PART_THUNDERX2, 0, 0, 0x80, 0x10},
{0, 0, 0, 0, 0, 0}
};
andrewUnsubmitted
Not Done
Inline Actions

Why is this needed?

andrew: Why is this needed?
pdk_semihalf.comAuthorUnsubmitted
Not Done
Inline Actions

Second socket have SATA controller on chip. It is not exposed on board, and have bad adresses (like SATA on first socket). Instead of fixing addresses we block it.

pdk_semihalf.com: Second socket have SATA controller on chip. It is not exposed on board, and have bad adresses…
int int
pci_host_generic_core_attach(device_t dev) pci_host_generic_core_attach(device_t dev)
{ {
struct generic_pcie_core_softc *sc; struct generic_pcie_core_softc *sc;
int error; int error;
int rid; int rid;
sc = device_get_softc(dev); sc = device_get_softc(dev);
Show All 37 Linespci_host_generic_core_attach(device_t dev)
} }
error = rman_init(&sc->io_rman); error = rman_init(&sc->io_rman);
if (error) { if (error) {
device_printf(dev, "rman_init() failed. error = %d\n", error); device_printf(dev, "rman_init() failed. error = %d\n", error);
return (error); return (error);
} }
pci_host_generic_apply_quirks(dev);
return (0); return (0);
} }
static void
pci_host_generic_apply_quirks(device_t dev)
{
struct pci_host_generic_quirk_entry *quirk;
quirk = pci_host_generic_quirks;
while (1) {
if (quirk->impl == 0)
break;
if (CPU_MATCH(CPU_IMPL_MASK | CPU_PART_MASK,
quirk->impl, quirk->part, quirk->var, quirk->rev) &&
quirk->func != NULL)
quirk->func(dev);
quirk++;
}
}
static uint32_t static uint32_t
generic_pcie_read_config(device_t dev, u_int bus, u_int slot, generic_pcie_read_config(device_t dev, u_int bus, u_int slot,
u_int func, u_int reg, int bytes) u_int func, u_int reg, int bytes)
{ {
struct generic_pcie_core_softc *sc; struct generic_pcie_core_softc *sc;
bus_space_handle_t h; bus_space_handle_t h;
bus_space_tag_t t; bus_space_tag_t t;
uint64_t offset; uint64_t offset;
uint32_t data; uint32_t data;
struct pci_host_generic_block_entry *block;
if ((bus > PCI_BUSMAX) || (slot > PCI_SLOTMAX) || if ((bus > PCI_BUSMAX) || (slot > PCI_SLOTMAX) ||
(func > PCI_FUNCMAX) || (reg > PCIE_REGMAX)) (func > PCI_FUNCMAX) || (reg > PCIE_REGMAX))
return (~0U); return (~0U);
block = pci_host_generic_blocked;
while (1) {
if (block->impl == 0)
break;
if (CPU_MATCH(CPU_IMPL_MASK | CPU_PART_MASK,
block->impl, block->part, block->var, block->rev) &&
block->bus == bus && block->slot == slot)
return (~0);
block++;
}
sc = device_get_softc(dev); sc = device_get_softc(dev);
offset = PCIE_ADDR_OFFSET(bus, slot, func, reg); offset = PCIE_ADDR_OFFSET(bus, slot, func, reg);
t = sc->bst; t = sc->bst;
h = sc->bsh; h = sc->bsh;
switch (bytes) { switch (bytes) {
case 1: case 1:
▲ Show 20 Lines • Show All 224 Lines • ▼ Show 20 Linesstatic device_method_t generic_pcie_methods[] = {
DEVMETHOD(pcib_read_config, generic_pcie_read_config), DEVMETHOD(pcib_read_config, generic_pcie_read_config),
DEVMETHOD(pcib_write_config, generic_pcie_write_config), DEVMETHOD(pcib_write_config, generic_pcie_write_config),
DEVMETHOD_END DEVMETHOD_END
}; };
DEFINE_CLASS_0(pcib, generic_pcie_core_driver, DEFINE_CLASS_0(pcib, generic_pcie_core_driver,
generic_pcie_methods, sizeof(struct generic_pcie_core_softc)); generic_pcie_methods, sizeof(struct generic_pcie_core_softc));
static void thunderx2_ahci_bar_quirk(device_t dev)
{
/*
* XXX:
* On ThunderX2, AHCI BAR2 address is wrong. It needs to precisely
* match the one described in datasheet. Fixup it unconditionally.
andrewUnsubmitted
Not Done
Inline Actions

Is there an erratum for this?

andrew: Is there an erratum for this?
pdk_semihalf.comAuthorUnsubmitted
Not Done
Inline Actions

There is no erratum. Without this quirk kernel is trying to access bad address.

pdk_semihalf.com: There is no erratum. Without this quirk kernel is trying to access bad address.
andrewUnsubmitted
Not Done
Inline Actions

Why is the kernel trying to access a bad address?

andrew: Why is the kernel trying to access a bad address?
pdk_semihalf.comAuthorUnsubmitted
Not Done
Inline Actions

Kernel is trying to attach ahci. Look at log (without quirk of course) below it contains aforementioned bad address:

ahci0: <AHCI SATA controller> mem 0x41d4100041d30000-0x41d4100041d3ffff,0x41d20000-0x41d2ffff at device 16.0 on pci0                
PDK: virtual=0xffff000165388000 size=65536 pa=41d20000                                                                              
PDK: virtual=0xffff000165398000 size=65536 pa=41d4100041d30000                                                                      
ahci0: PDK: msix_table_bar: 24                                                                                                      
ahci0: PDK: msix_table_res: 0xfffffd0001e25180                                                                                      
ahci0: attempting to allocate 4 MSI-X vectors (4 supported)                                                                         
ahci0: using IRQs 11-14 for MSI-X                                                                                                                                                                                                                                                                                               
panic: vm_fault_hold: fault on nofault entry, addr: 0xffff000165398000
pdk_semihalf.com: Kernel is trying to attach ahci. Look at log (without quirk of course) below it contains…
*/
if (device_get_unit(dev) == 0) {
device_printf(dev, "running AHCI BAR fixup\n");
PCIB_WRITE_CONFIG(dev, 0, 16, 0, 0x18, 0x01440000, 4);
PCIB_WRITE_CONFIG(dev, 0, 16, 0, 0x1c, 0x40, 4);
PCIB_WRITE_CONFIG(dev, 0, 16, 1, 0x18, 0x01450000, 4);
PCIB_WRITE_CONFIG(dev, 0, 16, 1, 0x1c, 0x40, 4);
}
}
andrewUnsubmitted
Not Done
Inline Actions

Shouldn't you be getting this from the ACPI mcfg table?

andrew: Shouldn't you be getting this from the ACPI mcfg table?
pdk_semihalf.comAuthorUnsubmitted
Not Done
Inline Actions

Yes, I should get this from MCFG, but it doesn't provide information about pci on second socket. Furthermore segment passed to acpi_enable_pcie is 0 in both cases.

pdk_semihalf.com: Yes, I should get this from MCFG, but it doesn't provide information about pci on second socket.
andrewUnsubmitted
Not Done
Inline Actions

Will this always be unit 16, or would it be better to look at other information to detect when it's an ecam device.

andrew: Will this always be unit 16, or would it be better to look at other information to detect when…
pdk_semihalf.comAuthorUnsubmitted
Not Done
Inline Actions

It will be always unit 16. I've checked it once again and I have an idea. I will use _BBN to get ecam number. I'll update patches today so stay tuned.

pdk_semihalf.com: It will be always unit 16. I've checked it once again and I have an idea. I will use _BBN to…