Differential D18774 Diff 53508 sys/dev/ntb/ntb_hw/ntb_hw_amd.c

Changeset View

Standalone View

sys/dev/ntb/ntb_hw/ntb_hw_amd.c

This file was added.

				/*-
				* This file is provided under a dual BSD/GPLv2 license. When using or
				* redistributing this file, you may do so under either license.
				*
				* GPL LICENSE SUMMARY
				*
				* Copyright (C) 2019 Advanced Micro Devices, Inc.
				*
				* This program is free software; you can redistribute it and/or modify
				* it under the terms of version 2 of the GNU General Public License as
				* published by the Free Software Foundation.
				*
				* BSD LICENSE
				*
				* Copyright (c) 2019 Advanced Micro Devices, Inc.
				*
				* Redistribution and use in source and binary forms, with or without
				* modification, are permitted provided that the following conditions
				* are met:
				* 1. Redistributions of source code must retain the above copyright
				* notice, this list of conditions and the following disclaimer.
				* 2. Redistributions in binary form must reproduce the above copyright
				* notice, this list of conditions and the following disclaimer in the
				* documentation and/or other materials provided with the distribution.
				* 3. Neither the name of AMD corporation nor the names of its
				* contributors may be used to endorse or promote products derived
				* from this software without specific prior written permission.
				*
				* THIS SOFTWARE IS PROVIDED BY THE AUTHOR AND CONTRIBUTORS ``AS IS'' AND
				* ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
				* IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
				* ARE DISCLAIMED. IN NO EVENT SHALL THE AUTHOR OR CONTRIBUTORS BE LIABLE
				* FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
				* DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
				* OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
				* HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
				* LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
				* OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
				* SUCH DAMAGE.
				*
				* Contact Information :
				* Rajesh Kumar <rajesh1.kumar@amd.com>
				*/

				/*
				* The Non-Transparent Bridge (NTB) is a device that allows you to connect
				* two or more systems using a PCI-e links, providing remote memory access.
				*
				* This module contains a driver for NTB hardware in AMD CPUs
				*
				* Much of the code in this module is shared with Linux. Any patches may
				* be picked up and redistributed in Linux with a dual GPL/BSD license.
				*/

				#include <sys/cdefs.h>
				__FBSDID("$FreeBSD$");

				#include <sys/param.h>
				#include <sys/kernel.h>
				#include <sys/systm.h>
				#include <sys/bus.h>
				#include <sys/malloc.h>
				#include <sys/module.h>
				#include <sys/rman.h>
				#include <sys/sbuf.h>
				#include <sys/sysctl.h>

				#include <vm/vm.h>
				#include <vm/pmap.h>

				#include <machine/bus.h>

				#include <dev/pci/pcireg.h>
				#include <dev/pci/pcivar.h>

				#include "ntb_hw_amd.h"
				#include "dev/ntb/ntb.h"

				MALLOC_DEFINE(M_AMD_NTB, "amd_ntb_hw", "amd_ntb_hw driver memory allocations");

				impUnsubmitted Done Inline Actions I'm not sure this is the right way to grab what you are grabbing. In the normal kernel build, all .h files are present in the current directory, or directly via -I lines. Having constructs like this leads to problems. imp: I'm not sure this is the right way to grab what you are grabbing. In the normal kernel build…
				rajeshaspAuthorUnsubmitted Done Inline Actions Changed it to "dev/ntb/ntb.h". Other hardware drivers shall also be changed to be in sync. @mav , can we make this change for other hardware drivers as well. rajeshasp: Changed it to "dev/ntb/ntb.h". Other hardware drivers shall also be changed to be in sync.
				struct pci_device_table amd_ntb_devs[] = {
				{ PCI_DEV(NTB_HW_AMD_VENDOR_ID, NTB_HW_AMD_DEVICE_ID),
				PCI_DESCR("AMD Non-Transparent Bridge") }
				};

				static unsigned g_amd_ntb_hw_debug_level;
				SYSCTL_UINT(_hw_ntb, OID_AUTO, debug_level, CTLFLAG_RWTUN,
				&g_amd_ntb_hw_debug_level, 0, "amd_ntb_hw log level -- higher is verbose");

				#define amd_ntb_printf(lvl, ...) do { \
				if (lvl <= g_amd_ntb_hw_debug_level) \
				device_printf(ntb->device, __VA_ARGS__); \
				} while (0)

				/*
				* AMD NTB INTERFACE ROUTINES
				*/
				static int
				amd_ntb_port_number(device_t dev)
				{
				struct amd_ntb_softc *ntb = device_get_softc(dev);

				amd_ntb_printf(1, "%s: conn_type %d\n", __func__, ntb->conn_type);

				switch (ntb->conn_type) {
				case NTB_CONN_PRI:
				return (NTB_PORT_PRI_USD);
				case NTB_CONN_SEC:
				return (NTB_PORT_SEC_DSD);
				default:
				impUnsubmitted Not Done Inline Actions This is 0 imp: This is 0
				break;
				}
				impUnsubmitted Not Done Inline Actions This is 1 imp: This is 1

				return (-EINVAL);
				}

				static int
				impUnsubmitted Done Inline Actions This a small positive integer of indeterminate value (which happens not to be 0 or 1 by chance). So you have overlapping ranges here which is dangerous. There's other places as well that have this construct. imp: This a small positive integer of indeterminate value (which happens not to be 0 or 1 by chance).
				rajeshaspAuthorUnsubmitted Done Inline Actions I assume, the concern here is returning positive error value, so that value could wrongly be interpreted as a valid value. If so, that's a valid concern and it's been changed to negative value now. rajeshasp: I assume, the concern here is returning positive error value, so that value could wrongly be…
				amd_ntb_peer_port_count(device_t dev)
				{
				struct amd_ntb_softc *ntb = device_get_softc(dev);

				amd_ntb_printf(1, "%s: peer cnt %d\n", __func__, NTB_DEF_PEER_CNT);
				return (NTB_DEF_PEER_CNT);
				}

				static int
				amd_ntb_peer_port_number(device_t dev, int pidx)
				{
				struct amd_ntb_softc *ntb = device_get_softc(dev);

				amd_ntb_printf(1, "%s: pidx %d conn type %d\n",
				__func__, pidx, ntb->conn_type);

				if (pidx != NTB_DEF_PEER_IDX)
				return (-EINVAL);

				switch (ntb->conn_type) {
				case NTB_CONN_PRI:
				return (NTB_PORT_SEC_DSD);
				case NTB_CONN_SEC:
				return (NTB_PORT_PRI_USD);
				default:
				break;
				}

				return (-EINVAL);
				}

				static int
				amd_ntb_peer_port_idx(device_t dev, int port)
				{
				struct amd_ntb_softc *ntb = device_get_softc(dev);
				int peer_port;

				peer_port = amd_ntb_peer_port_number(dev, NTB_DEF_PEER_IDX);

				amd_ntb_printf(1, "%s: port %d peer_port %d\n",
				__func__, port, peer_port);

				if (peer_port == -EINVAL \|\| port != peer_port)
				return (-EINVAL);

				return (0);
				}

				/*
				* AMD NTB INTERFACE - LINK ROUTINES
				*/
				static inline int
				amd_link_is_up(struct amd_ntb_softc *ntb)
				{

				amd_ntb_printf(2, "%s: peer_sta 0x%x cntl_sta 0x%x\n",
				__func__, ntb->peer_sta, ntb->cntl_sta);

				if (!ntb->peer_sta)
				return (NTB_LNK_STA_ACTIVE(ntb->cntl_sta));

				return (0);
				}

				static inline enum ntb_speed
				amd_ntb_link_sta_speed(struct amd_ntb_softc *ntb)
				{

				if (!amd_link_is_up(ntb))
				return (NTB_SPEED_NONE);

				return (NTB_LNK_STA_SPEED(ntb->lnk_sta));
				}

				static inline enum ntb_width
				amd_ntb_link_sta_width(struct amd_ntb_softc *ntb)
				{

				if (!amd_link_is_up(ntb))
				return (NTB_WIDTH_NONE);

				return (NTB_LNK_STA_WIDTH(ntb->lnk_sta));
				}

				static bool
				amd_ntb_link_is_up(device_t dev, enum ntb_speed speed, enum ntb_width width)
				{
				struct amd_ntb_softc *ntb = device_get_softc(dev);

				if (speed != NULL)
				*speed = amd_ntb_link_sta_speed(ntb);
				if (width != NULL)
				*width = amd_ntb_link_sta_width(ntb);

				return (amd_link_is_up(ntb));
				}

				static int
				amd_ntb_link_enable(device_t dev, enum ntb_speed max_speed,
				enum ntb_width max_width)
				{
				struct amd_ntb_softc *ntb = device_get_softc(dev);
				uint32_t ntb_ctl;

				amd_ntb_printf(1, "%s: int_mask 0x%x conn_type %d\n",
				__func__, ntb->int_mask, ntb->conn_type);

				amd_init_side_info(ntb);

				/* Enable event interrupt */
				ntb->int_mask &= ~AMD_EVENT_INTMASK;
				amd_ntb_reg_write(4, AMD_INTMASK_OFFSET, ntb->int_mask);

				if (ntb->conn_type == NTB_CONN_SEC)
				return (EINVAL);

				amd_ntb_printf(0, "%s: Enabling Link.\n", __func__);

				ntb_ctl = amd_ntb_reg_read(4, AMD_CNTL_OFFSET);
				ntb_ctl \|= (PMM_REG_CTL \| SMM_REG_CTL);
				amd_ntb_printf(1, "%s: ntb_ctl 0x%x\n", __func__, ntb_ctl);
				amd_ntb_reg_write(4, AMD_CNTL_OFFSET, ntb_ctl);

				return (0);
				}

				static int
				amd_ntb_link_disable(device_t dev)
				{
				struct amd_ntb_softc *ntb = device_get_softc(dev);
				uint32_t ntb_ctl;

				amd_ntb_printf(1, "%s: int_mask 0x%x conn_type %d\n",
				__func__, ntb->int_mask, ntb->conn_type);

				amd_deinit_side_info(ntb);

				/* Disable event interrupt */
				ntb->int_mask \|= AMD_EVENT_INTMASK;
				amd_ntb_reg_write(4, AMD_INTMASK_OFFSET, ntb->int_mask);

				if (ntb->conn_type == NTB_CONN_SEC)
				return (EINVAL);

				amd_ntb_printf(0, "%s: Disabling Link.\n", __func__);

				ntb_ctl = amd_ntb_reg_read(4, AMD_CNTL_OFFSET);
				ntb_ctl &= ~(PMM_REG_CTL \| SMM_REG_CTL);
				amd_ntb_printf(1, "%s: ntb_ctl 0x%x\n", __func__, ntb_ctl);
				amd_ntb_reg_write(4, AMD_CNTL_OFFSET, ntb_ctl);

				return (0);
				}

				/*
				* AMD NTB memory window routines
				*/
				static uint8_t
				amd_ntb_mw_count(device_t dev)
				{
				struct amd_ntb_softc *ntb = device_get_softc(dev);

				return (ntb->mw_count);
				}

				static int
				amd_ntb_mw_get_range(device_t dev, unsigned mw_idx, vm_paddr_t *base,
				caddr_t vbase, size_t size, size_t align, size_t align_size,
				bus_addr_t *plimit)
				{
				struct amd_ntb_softc *ntb = device_get_softc(dev);
				struct amd_ntb_pci_bar_info *bar_info;

				if (mw_idx < 0 \|\| mw_idx >= ntb->mw_count)
				return (EINVAL);

				bar_info = &ntb->bar_info[mw_idx+1];

				if (base != NULL)
				*base = bar_info->pbase;

				cemUnsubmitted Done Inline Actions style nit: a space goes between `if` and `(`. cem: style nit: a space goes between `if` and `(`.
				rajeshaspAuthorUnsubmitted Done Inline Actions Also, checked for other similar occurrences and corrected it. rajeshasp: Also, checked for other similar occurrences and corrected it.
				if (vbase != NULL)
				*vbase = bar_info->vbase;

				if (align != NULL)
				*align = 0x1000;

				mavUnsubmitted Done Inline Actions Just to be sure, does the hardware allows 1MB aligned windows for any window sizes? Not aligned to bigger window sizes? Neither Intel nor PLX can do that. For PLX I had to use A-LUT translation table to reduce this constraint. mav: Just to be sure, does the hardware allows 1MB aligned windows for any window sizes? Not…
				rajeshaspAuthorUnsubmitted Done Inline Actions @mav, you are right. HW doesn't allow 1M aligned. It allow a granularity range. The minimum granularity supported by this hardware is 4K aligned, which is what Linux provides. Made the same changes to return 4K aligned now in FreeBSD. Just curious, when you say "Intel and PLX can't do that", I assume those hardware doesn't allow higher size aligned (like 1M)? Is that right? But in code, I see the comments like the alignment should be BAR sized. Also, A-LUT is there to support small size granularity. So, can you please clarify what you mean? rajeshasp: @mav, you are right. HW doesn't allow 1M aligned. It allow a granularity range. The minimum…
				mavUnsubmitted Not Done Inline Actions Your change to 4K is actually opposite of what I expected. So I'd recommend to do double check that. What I am saying is that both Intel and PLX NTB (without A-LUT) require translation address to be multiple of window (PCI BAR) size, i.e. NTB window of 1MB must be aligned to 1MB on both sides, and window of 64GB has to be aligned to 64GB, that is actually quite inconvenient. In case of PLX there is additional A-LUT table of 256 entries, which allow single window to be translated as 256 separate chunks, so that 64GB window may have alignment of 65536/256 => 256MB, that is much nicer. mav: Your change to 4K is actually opposite of what I expected. So I'd recommend to do double check…
				rajeshaspAuthorUnsubmitted Done Inline Actions I will double check about this part and update about it. rajeshasp: I will double check about this part and update about it.
				if (size != NULL)
				cemUnsubmitted Done Inline Actions Usual style for indentation on subsequent lines is 4 spaces. We have a style(9) manual page for more examples / specifics. cem: Usual style for indentation on subsequent lines is 4 spaces. We have a style(9) manual page…
				rajeshaspAuthorUnsubmitted Done Inline Actions Also, checked for other similar occurrences and corrected it. rajeshasp: Also, checked for other similar occurrences and corrected it.
				*size = bar_info->size;

				if (align_size != NULL)
				*align_size = 1;

				if (plimit != NULL) {
				if (mw_idx != 0)
				*plimit = BUS_SPACE_MAXADDR;
				else
				*plimit = BUS_SPACE_MAXADDR_32BIT;
				}

				amd_ntb_printf(1, "%s: mw %d padd %p vadd %p psize 0x%lx "
				"align 0x%lx asize 0x%lx alimit %p\n", __func__, mw_idx,
				(void )base, (void )vbase, (uint64_t)size, (uint64_t)align,
				(uint64_t)align_size, (void )*plimit);

				return (0);
				}

				static int
				amd_ntb_mw_set_trans(device_t dev, unsigned mw_idx, bus_addr_t addr, size_t size)
				{
				struct amd_ntb_softc *ntb = device_get_softc(dev);
				struct amd_ntb_pci_bar_info *bar_info;
				uint64_t reg_val;

				if (mw_idx < 0 \|\| mw_idx >= ntb->mw_count)
				return (EINVAL);

				bar_info = &ntb->bar_info[mw_idx+1];

				/* make sure the range fits in the usable mw size */
				if (size > bar_info->size) {
				amd_ntb_printf(0, "%s: size 0x%x greater than mw_size 0x%x\n",
				__func__, (uint32_t)size, (uint32_t)bar_info->size);
				return (EINVAL);
				}

				amd_ntb_printf(1, "%s: mw %d mw_size 0x%x size 0x%x base %p\n",
				__func__, mw_idx, (uint32_t)bar_info->size,
				(uint32_t)size, (void *)bar_info->pci_bus_handle);

				/* set and verify setting the translation address */
				amd_ntb_printf(0, "%s: mw %d xlat_off 0x%x cur_val 0x%lx addr %p\n",
				__func__, mw_idx, bar_info->xlat_off,
				amd_ntb_peer_reg_read(8, bar_info->xlat_off), (void *)addr);
				amd_ntb_peer_reg_write(8, bar_info->xlat_off, (uint64_t)addr);
				reg_val = amd_ntb_peer_reg_read(8, bar_info->xlat_off);
				if (reg_val != addr) {
				amd_ntb_printf(0, "%s: Failed writing xlat addr %p reg_val 0x%lx\n",
				mavUnsubmitted Not Done Inline Actions I am curios why those checks are needed? If hardware has constraints on translation address, instead of failing here you should better report the constraint via align above. You may check if there is a reason, but I'd prefer it to be exception rather then rule. mav: I am curios why those checks are needed? If hardware has constraints on translation address…
				rajeshaspAuthorUnsubmitted Done Inline Actions In AMD hardware, these registers are read-only when link is not enabled. So, when a client (ntb_transport) is loaded, first it calls set_trans to write these register and then does link enable. So these checks are there to capture the read-only scenario and return without proceeding furthur. Also inline with this, needed a clarification. In client driver detach path, link disable happens before clear_trans(from free_mws). Is that valid? Shouldn't it be clearing the translate registers, then link disable and then free the mw resources? Because of this, we have the old values retained in some scenarios until the next set_trans happens to overwrite. Functionally, it is not disturbing. But looking for clarity on this. rajeshasp: In AMD hardware, these registers are read-only when link is not enabled. So, when a client…
				mavUnsubmitted Not Done Inline Actions You may see, that early window allocation was added to try avoid problems with allocation of significant piece of physically contiguous memory if it take a lot of system run time before the link first established. After the link is established, the code will check and try to adjust allocation, if needed. Neither Intel nor PLX had the problem with register writing you are describing, so the code may indeed require some tuning. Just consider that error reported there during pre-allocation will free the allocated memory window, that may give you back the problem I described later. mav: You may see, that early window allocation was added to try avoid problems with allocation of…
				rajeshaspAuthorUnsubmitted Done Inline Actions Yes, we might run into the problem of not getting contiguous physical memory. But problem here is, in ntb_transport when link_up event happens and when the code tries to adjust allocation, it's seeing that (mw->xlat_size == xlat_size), and returning without programming the xlat and limit registers (set_trans will not be called). So, if that check in AMD driver needs to be removed. I could see two possible ways to handle this. I am not sure writing the xlat and limit registers once again would be a problem with Intel and PLX. If not, can we re-write those registers again when link up happens? In Linux, set_trans happens only during Link event. So, can we have the same approach here? Inline with this, may be a silly question. HW Link up happens right after module loading. So do we really face trouble in getting the physically contiguous memory during link events? But wondering about the use-case for the probability of this issue? Any comments? or any better way we can go about here? rajeshasp: Yes, we might run into the problem of not getting contiguous physical memory. But problem here…
				mavUnsubmitted Not Done Inline Actions I think we could split memory allocation from setting translation address in ntb_transport, and still try to preallocate it on driver load, while set translation address only on link set up. What worries me though, is that if as you say registers are not writable when link is down, then what happen when link is back up? Will they be reset to defaults with window being disabled until configured, or keep their values, and letting remote side access possibly repurposed memory? What's about link up delay, the reason is simple -- the other node is physically not present, powered off, or at least not booted at the time. It may boot first time days or even months after the first one, when the first one has no physically contiguous memory any more. mav: I think we could split memory allocation from setting translation address in ntb_transport, and…
				rajeshaspAuthorUnsubmitted Done Inline Actions Splitting memory allocation and setting xlat address would be good. To clarify, those registers are read-only only until link is enabled (not till link up). Link enable happens during driver load (but after ntb_set_mw to program the xlat and limit registers, which is the problem we have now). So, if splitting should be fine, we can allocate the memory before link enable and program the register after link enable. And we can have both memory reallocation and programming during link up (as we have currently), but just having them split. These registers will be RW until link is disabled (which happens during driver unload). So, link up/down will not disturb those registers or mw window access differently (unless mw size changes and needs reallocation during link up). If we are ok with this approach, I shall make the necessary changes and submit the next patch. I am yet to get the clarity on the alignment part. Will try to get that asap. rajeshasp: Splitting memory allocation and setting xlat address would be good. To clarify, those…
				__func__, (void *)addr, reg_val);
				amd_ntb_peer_reg_write(8, bar_info->xlat_off, (uint64_t)0);
				return (EIO);
				}

				/* set and verify setting the limit */
				if (mw_idx != 0) {
				amd_ntb_printf(1, "%s: limit_off 0x%x cur_val 0x%lx limit 0x%x\n",
				__func__, bar_info->limit_off,
				amd_ntb_peer_reg_read(8, bar_info->limit_off), (uint32_t)size);
				amd_ntb_reg_write(8, bar_info->limit_off, (uint64_t)size);
				reg_val = amd_ntb_reg_read(8, bar_info->limit_off);
				if (reg_val != size) {
				amd_ntb_printf(0, "%s: Failed writing mw %d limit 0x%x "
				"reg_val 0x%lx\n", __func__, mw_idx, (uint32_t)size,
				reg_val);
				amd_ntb_reg_write(8, bar_info->limit_off,
				(uint64_t)bar_info->pci_bus_handle);
				amd_ntb_peer_reg_write(8, bar_info->xlat_off, 0);
				return (EIO);
				}
				} else {
				amd_ntb_printf(1, "%s: limit_off 0x%x cur_val 0x%x limit 0x%x\n",
				__func__, bar_info->limit_off,
				amd_ntb_peer_reg_read(4, bar_info->limit_off), (uint32_t)size);
				amd_ntb_reg_write(4, bar_info->limit_off, (uint64_t)size);
				reg_val = amd_ntb_reg_read(4, bar_info->limit_off);
				if (reg_val != size) {
				amd_ntb_printf(0, "%s: Failed writing mw %d limit 0x%x "
				mavUnsubmitted Not Done Inline Actions I am curios why those checks are needed? If hardware has constraints on window size, instead of failing here you should better report the constraint via align_size above. mav: I am curios why those checks are needed? If hardware has constraints on window size, instead…
				rajeshaspAuthorUnsubmitted Done Inline Actions This is for the same reason why we have the check for xlat registers above. rajeshasp: This is for the same reason why we have the check for xlat registers above.
				"reg_val 0x%lx\n", __func__, mw_idx, (uint32_t)size,
				reg_val);
				amd_ntb_reg_write(4, bar_info->limit_off,
				(uint32_t)bar_info->pci_bus_handle);
				amd_ntb_peer_reg_write(4, bar_info->xlat_off, 0);
				return (EIO);
				}
				}

				return (0);
				}

				static int
				amd_ntb_mw_clear_trans(device_t dev, unsigned mw_idx)
				{
				struct amd_ntb_softc *ntb = device_get_softc(dev);

				amd_ntb_printf(1, "%s: mw_idx %d\n", __func__, mw_idx);

				if (mw_idx < 0 \|\| mw_idx >= ntb->mw_count)
				return (EINVAL);

				return (amd_ntb_mw_set_trans(dev, mw_idx, 0, 0));
				}

				static int
				amd_ntb_mw_set_wc(device_t dev, unsigned int mw_idx, vm_memattr_t mode)
				{
				struct amd_ntb_softc *ntb = device_get_softc(dev);
				struct amd_ntb_pci_bar_info *bar_info;
				int rc;

				if (mw_idx < 0 \|\| mw_idx >= ntb->mw_count)
				return (EINVAL);

				bar_info = &ntb->bar_info[mw_idx+1];
				if (mode == bar_info->map_mode)
				return (0);

				rc = pmap_change_attr((vm_offset_t)bar_info->vbase, bar_info->size, mode);
				if (rc == 0)
				bar_info->map_mode = mode;

				return (rc);
				}

				static int
				amd_ntb_mw_get_wc(device_t dev, unsigned mw_idx, vm_memattr_t *mode)
				{
				struct amd_ntb_softc *ntb = device_get_softc(dev);
				struct amd_ntb_pci_bar_info *bar_info;

				amd_ntb_printf(1, "%s: mw_idx %d\n", __func__, mw_idx);

				if (mw_idx < 0 \|\| mw_idx >= ntb->mw_count)
				return (EINVAL);

				bar_info = &ntb->bar_info[mw_idx+1];
				*mode = bar_info->map_mode;

				return (0);
				}

				/*
				* AMD NTB doorbell routines
				*/
				static int
				amd_ntb_db_vector_count(device_t dev)
				{
				struct amd_ntb_softc *ntb = device_get_softc(dev);

				amd_ntb_printf(1, "%s: db_count 0x%x\n", __func__, ntb->db_count);

				return (ntb->db_count);
				}

				static uint64_t
				amd_ntb_db_valid_mask(device_t dev)
				{
				struct amd_ntb_softc *ntb = device_get_softc(dev);

				amd_ntb_printf(1, "%s: db_valid_mask 0x%x\n",
				__func__, ntb->db_valid_mask);

				return (ntb->db_valid_mask);
				}

				static uint64_t
				amd_ntb_db_vector_mask(device_t dev, uint32_t vector)
				{
				struct amd_ntb_softc *ntb = device_get_softc(dev);

				amd_ntb_printf(1, "%s: vector %d db_count 0x%x db_valid_mask 0x%x\n",
				__func__, vector, ntb->db_count, ntb->db_valid_mask);

				if (vector < 0 \|\| vector >= ntb->db_count)
				return (0);

				return (ntb->db_valid_mask & (1 << vector));
				}

				static uint64_t
				amd_ntb_db_read(device_t dev)
				{
				struct amd_ntb_softc *ntb = device_get_softc(dev);
				uint64_t dbstat_off;

				dbstat_off = (uint64_t)amd_ntb_reg_read(2, AMD_DBSTAT_OFFSET);

				amd_ntb_printf(1, "%s: dbstat_off 0x%lx\n", __func__, dbstat_off);

				return (dbstat_off);
				}

				static void
				amd_ntb_db_clear(device_t dev, uint64_t db_bits)
				{
				struct amd_ntb_softc *ntb = device_get_softc(dev);

				amd_ntb_printf(1, "%s: db_bits 0x%lx\n", __func__, db_bits);
				amd_ntb_reg_write(2, AMD_DBSTAT_OFFSET, (uint16_t)db_bits);
				}

				static void
				amd_ntb_db_set_mask(device_t dev, uint64_t db_bits)
				{
				struct amd_ntb_softc *ntb = device_get_softc(dev);

				DB_MASK_LOCK(ntb);
				amd_ntb_printf(1, "%s: db_mask 0x%x db_bits 0x%lx\n",
				__func__, ntb->db_mask, db_bits);

				ntb->db_mask \|= db_bits;
				amd_ntb_reg_write(2, AMD_DBMASK_OFFSET, ntb->db_mask);
				DB_MASK_UNLOCK(ntb);
				}

				static void
				amd_ntb_db_clear_mask(device_t dev, uint64_t db_bits)
				{
				struct amd_ntb_softc *ntb = device_get_softc(dev);

				DB_MASK_LOCK(ntb);
				amd_ntb_printf(1, "%s: db_mask 0x%x db_bits 0x%lx\n",
				__func__, ntb->db_mask, db_bits);

				ntb->db_mask &= ~db_bits;
				amd_ntb_reg_write(2, AMD_DBMASK_OFFSET, ntb->db_mask);
				DB_MASK_UNLOCK(ntb);
				}

				static void
				amd_ntb_peer_db_set(device_t dev, uint64_t db_bits)
				{
				struct amd_ntb_softc *ntb = device_get_softc(dev);

				amd_ntb_printf(1, "%s: db_bits 0x%lx\n", __func__, db_bits);
				amd_ntb_reg_write(2, AMD_DBREQ_OFFSET, (uint16_t)db_bits);
				}

				/*
				* AMD NTB scratchpad routines
				*/
				static uint8_t
				amd_ntb_spad_count(device_t dev)
				{
				struct amd_ntb_softc *ntb = device_get_softc(dev);

				amd_ntb_printf(1, "%s: spad_count 0x%x\n", __func__, ntb->spad_count);

				return (ntb->spad_count);
				}

				static int
				amd_ntb_spad_read(device_t dev, unsigned int idx, uint32_t *val)
				{
				struct amd_ntb_softc *ntb = device_get_softc(dev);
				uint32_t offset;

				amd_ntb_printf(2, "%s: idx %d\n", __func__, idx);

				if (idx < 0 \|\| idx >= ntb->spad_count)
				return (EINVAL);

				offset = ntb->self_spad + (idx << 2);
				*val = amd_ntb_reg_read(4, AMD_SPAD_OFFSET + offset);
				amd_ntb_printf(2, "%s: offset 0x%x val 0x%x\n", __func__, offset, *val);

				return (0);
				}

				static int
				amd_ntb_spad_write(device_t dev, unsigned int idx, uint32_t val)
				{
				struct amd_ntb_softc *ntb = device_get_softc(dev);
				uint32_t offset;

				amd_ntb_printf(2, "%s: idx %d\n", __func__, idx);

				if (idx < 0 \|\| idx >= ntb->spad_count)
				return (EINVAL);

				offset = ntb->self_spad + (idx << 2);
				amd_ntb_reg_write(4, AMD_SPAD_OFFSET + offset, val);
				amd_ntb_printf(2, "%s: offset 0x%x val 0x%x\n", __func__, offset, val);

				return (0);
				}

				static void
				amd_ntb_spad_clear(struct amd_ntb_softc *ntb)
				{
				uint8_t i;

				for (i = 0; i < ntb->spad_count; i++)
				amd_ntb_spad_write(ntb->device, i, 0);
				}

				static int
				amd_ntb_peer_spad_read(device_t dev, unsigned int idx, uint32_t *val)
				{
				struct amd_ntb_softc *ntb = device_get_softc(dev);
				uint32_t offset;

				amd_ntb_printf(2, "%s: idx %d\n", __func__, idx);

				if (idx < 0 \|\| idx >= ntb->spad_count)
				return (EINVAL);

				offset = ntb->peer_spad + (idx << 2);
				*val = amd_ntb_reg_read(4, AMD_SPAD_OFFSET + offset);
				amd_ntb_printf(2, "%s: offset 0x%x val 0x%x\n", __func__, offset, *val);

				return (0);
				}

				static int
				amd_ntb_peer_spad_write(device_t dev, unsigned int idx, uint32_t val)
				{
				struct amd_ntb_softc *ntb = device_get_softc(dev);
				uint32_t offset;

				amd_ntb_printf(2, "%s: idx %d\n", __func__, idx);

				if (idx < 0 \|\| idx >= ntb->spad_count)
				return (EINVAL);

				offset = ntb->peer_spad + (idx << 2);
				amd_ntb_reg_write(4, AMD_SPAD_OFFSET + offset, val);
				amd_ntb_printf(2, "%s: offset 0x%x val 0x%x\n", __func__, offset, val);

				return (0);
				}


				/*
				* AMD NTB INIT
				*/
				static int
				amd_ntb_hw_info_handler(SYSCTL_HANDLER_ARGS)
				{
				struct amd_ntb_softc* ntb = arg1;
				struct sbuf *sb;
				int rc = 0;

				sb = sbuf_new_for_sysctl(NULL, NULL, 4096, req);
				if (sb == NULL)
				return (sb->s_error);

				sbuf_printf(sb, "NTB AMD Hardware info:\n\n");
				sbuf_printf(sb, "AMD NTB side: %s\n",
				(ntb->conn_type == NTB_CONN_PRI)? "PRIMARY" : "SECONDARY");
				sbuf_printf(sb, "AMD LNK STA: 0x%#06x\n", ntb->lnk_sta);

				cemUnsubmitted Done Inline Actions Starting a string sysctl with a newline is unconventional cem: Starting a string sysctl with a newline is unconventional
				if (!amd_link_is_up(ntb))
				sbuf_printf(sb, "AMD Link Status: Down\n");
				else {
				sbuf_printf(sb, "AMD Link Status: Up\n");
				sbuf_printf(sb, "AMD Link Speed: PCI-E Gen %u\n",
				NTB_LNK_STA_SPEED(ntb->lnk_sta));
				sbuf_printf(sb, "AMD Link Width: PCI-E Width %u\n",
				NTB_LNK_STA_WIDTH(ntb->lnk_sta));
				}

				sbuf_printf(sb, "AMD Memory window count: %d\n",
				ntb->mw_count);
				sbuf_printf(sb, "AMD Spad count: %d\n",
				ntb->spad_count);
				sbuf_printf(sb, "AMD Doorbell count: %d\n",
				ntb->db_count);
				sbuf_printf(sb, "AMD MSI-X vec count: %d\n\n",
				ntb->msix_vec_count);
				sbuf_printf(sb, "AMD Doorbell valid mask: 0x%x\n",
				ntb->db_valid_mask);
				sbuf_printf(sb, "AMD Doorbell Mask: 0x%x\n",
				amd_ntb_reg_read(4, AMD_DBMASK_OFFSET));
				sbuf_printf(sb, "AMD Doorbell: 0x%x\n",
				amd_ntb_reg_read(4, AMD_DBSTAT_OFFSET));
				sbuf_printf(sb, "AMD NTB Incoming XLAT: \n");
				sbuf_printf(sb, "AMD XLAT1: 0x%lx\n",
				amd_ntb_peer_reg_read(8, AMD_BAR1XLAT_OFFSET));
				sbuf_printf(sb, "AMD XLAT23: 0x%lx\n",
				amd_ntb_peer_reg_read(8, AMD_BAR23XLAT_OFFSET));
				sbuf_printf(sb, "AMD XLAT45: 0x%lx\n",
				amd_ntb_peer_reg_read(8, AMD_BAR45XLAT_OFFSET));
				sbuf_printf(sb, "AMD LMT1: 0x%x\n",
				amd_ntb_reg_read(4, AMD_BAR1LMT_OFFSET));
				sbuf_printf(sb, "AMD LMT23: 0x%lx\n",
				amd_ntb_reg_read(8, AMD_BAR23LMT_OFFSET));
				sbuf_printf(sb, "AMD LMT45: 0x%lx\n",
				amd_ntb_reg_read(8, AMD_BAR45LMT_OFFSET));

				rc = sbuf_finish(sb);
				sbuf_delete(sb);
				return (rc);
				}

				static void
				amd_ntb_sysctl_init(struct amd_ntb_softc *ntb)
				{
				struct sysctl_oid_list *globals;
				struct sysctl_ctx_list *ctx;

				ctx = device_get_sysctl_ctx(ntb->device);
				globals = SYSCTL_CHILDREN(device_get_sysctl_tree(ntb->device));

				SYSCTL_ADD_PROC(ctx, globals, OID_AUTO, "info",
				CTLTYPE_STRING \| CTLFLAG_RD \| CTLFLAG_MPSAFE, ntb, 0,
				amd_ntb_hw_info_handler, "A", "AMD NTB HW Information");
				}

				impUnsubmitted Not Done Inline Actions So ntb doesn't ever change? If that's true, this is OK. If it changes, there's no locking in amd_ntb_hw_info_handler. imp:* So *ntb doesn't ever change? If that's true, this is OK. If it changes, there's no locking in…
				rajeshaspAuthorUnsubmitted Done Inline Actions Other than lnk_sta, rest of the ntb structure elements accessed are constants. Other values which could change, are being read from registers (using bus_space_read_X calls), which I believe is atomic and doesn't need any locking. "lnk_sta" is also changed only during link events. Please let me know if that needs a lock. rajeshasp: Other than lnk_sta, rest of the ntb structure elements accessed are constants. Other values…
				/*
				* Polls the HW link status register(s); returns true if something has changed.
				*/
				static bool
				amd_ntb_poll_link(struct amd_ntb_softc *ntb)
				{
				uint32_t fullreg, reg, stat;

				fullreg = amd_ntb_peer_reg_read(4, AMD_SIDEINFO_OFFSET);
				reg = fullreg & NTB_LIN_STA_ACTIVE_BIT;

				if (reg == ntb->cntl_sta)
				return (false);

				amd_ntb_printf(0, "%s: SIDEINFO reg_val = 0x%x cntl_sta 0x%x\n",
				__func__, fullreg, ntb->cntl_sta);

				ntb->cntl_sta = reg;

				stat = pci_read_config(ntb->device, AMD_LINK_STATUS_OFFSET, 4);

				amd_ntb_printf(0, "%s: LINK_STATUS stat = 0x%x lnk_sta 0x%x.\n",
				__func__, stat, ntb->lnk_sta);

				ntb->lnk_sta = stat;

				return (true);
				}

				static void
				amd_link_hb(void *arg)
				{
				struct amd_ntb_softc *ntb = arg;

				if (amd_ntb_poll_link(ntb))
				ntb_link_event(ntb->device);

				if (!amd_link_is_up(ntb)) {
				callout_reset(&ntb->hb_timer, AMD_LINK_HB_TIMEOUT,
				amd_link_hb, ntb);
				} else {
				callout_reset(&ntb->hb_timer, (AMD_LINK_HB_TIMEOUT * 10),
				amd_link_hb, ntb);
				}
				}

				static void
				amd_ntb_interrupt(struct amd_ntb_softc *ntb, uint16_t vec)
				{
				if (vec < AMD_DB_CNT)
				ntb_db_event(ntb->device, vec);
				else
				amd_ntb_printf(0, "Invalid vector %d\n", vec);
				}

				static void
				amd_ntb_vec_isr(void *arg)
				{
				struct amd_ntb_vec *nvec = arg;

				amd_ntb_interrupt(nvec->ntb, nvec->num);
				}

				static void
				amd_ntb_irq_isr(void *arg)
				{
				/* If we couldn't set up MSI-X, we only have the one vector. */
				amd_ntb_interrupt(arg, 0);
				}

				static void
				amd_init_side_info(struct amd_ntb_softc *ntb)
				{
				unsigned int reg;

				reg = amd_ntb_reg_read(4, AMD_SIDEINFO_OFFSET);
				if (!(reg & AMD_SIDE_READY)) {
				reg \|= AMD_SIDE_READY;
				amd_ntb_reg_write(4, AMD_SIDEINFO_OFFSET, reg);
				}
				reg = amd_ntb_reg_read(4, AMD_SIDEINFO_OFFSET);
				}

				static void
				amd_deinit_side_info(struct amd_ntb_softc *ntb)
				{
				unsigned int reg;

				reg = amd_ntb_reg_read(4, AMD_SIDEINFO_OFFSET);
				if (reg & AMD_SIDE_READY) {
				reg &= ~AMD_SIDE_READY;
				amd_ntb_reg_write(4, AMD_SIDEINFO_OFFSET, reg);
				amd_ntb_reg_read(4, AMD_SIDEINFO_OFFSET);
				}
				}

				static int
				amd_ntb_setup_isr(struct amd_ntb_softc *ntb, uint16_t num_vectors, bool msi,
				bool intx)
				{
				uint16_t i;
				int flags = 0, rc = 0;

				flags \|= RF_ACTIVE;
				if (intx)
				flags \|= RF_SHAREABLE;

				for (i = 0; i < num_vectors; i++) {

				/* RID should be 0 for intx */
				if (intx)
				ntb->int_info[i].rid = i;
				else
				ntb->int_info[i].rid = i + 1;

				ntb->int_info[i].res = bus_alloc_resource_any(ntb->device,
				SYS_RES_IRQ, &ntb->int_info[i].rid, flags);
				if (ntb->int_info[i].res == NULL) {
				amd_ntb_printf(0, "bus_alloc_resource IRQ failed\n");
				return (ENOMEM);
				}

				ntb->int_info[i].tag = NULL;
				ntb->allocated_interrupts++;

				if (msi \|\| intx) {
				rc = bus_setup_intr(ntb->device, ntb->int_info[i].res,
				INTR_MPSAFE \| INTR_TYPE_MISC, NULL, amd_ntb_irq_isr,
				ntb, &ntb->int_info[i].tag);
				} else {
				rc = bus_setup_intr(ntb->device, ntb->int_info[i].res,
				INTR_MPSAFE \| INTR_TYPE_MISC, NULL, amd_ntb_vec_isr,
				&ntb->msix_vec[i], &ntb->int_info[i].tag);
				}

				if (rc != 0) {
				amd_ntb_printf(0, "bus_setup_intr %d failed\n", i);
				return (ENXIO);
				}
				}

				return (0);
				}

				static int
				amd_ntb_create_msix_vec(struct amd_ntb_softc *ntb, uint32_t max_vectors)
				{
				uint8_t i;

				ntb->msix_vec = malloc(max_vectors * sizeof(*ntb->msix_vec), M_AMD_NTB,
				M_ZERO \| M_WAITOK);

				for (i = 0; i < max_vectors; i++) {
				ntb->msix_vec[i].num = i;
				ntb->msix_vec[i].ntb = ntb;
				}

				cemUnsubmitted Done Inline Actions `M_WAITOK` allocations cannot fail in FreeBSD. cem: `M_WAITOK` allocations cannot fail in FreeBSD.
				rajeshaspAuthorUnsubmitted Done Inline Actions Removed the check for NULL rajeshasp: Removed the check for NULL
				return (0);
				}

				static void
				amd_ntb_free_msix_vec(struct amd_ntb_softc *ntb)
				{
				if (ntb->msix_vec_count) {
				pci_release_msi(ntb->device);
				ntb->msix_vec_count = 0;
				}

				if (ntb->msix_vec != NULL) {
				free(ntb->msix_vec, M_AMD_NTB);
				ntb->msix_vec = NULL;
				}
				}

				static int
				amd_ntb_init_isr(struct amd_ntb_softc *ntb)
				{
				uint32_t supported_vectors, num_vectors;
				bool msi = false, intx = false;
				int rc = 0;

				ntb->db_mask = ntb->db_valid_mask;

				rc = amd_ntb_create_msix_vec(ntb, AMD_MSIX_VECTOR_CNT);
				if (rc != 0) {
				amd_ntb_printf(0, "Error creating msix vectors: %d\n", rc);
				return (ENOMEM);
				}

				/*
				* Check the number of MSI-X message supported by the device.
				* Minimum necessary MSI-X message count should be equal to db_count
				*/
				supported_vectors = pci_msix_count(ntb->device);
				num_vectors = MIN(supported_vectors, ntb->db_count);
				if (num_vectors < ntb->db_count) {
				amd_ntb_printf(0, "No minimum msix: supported %d db %d\n",
				supported_vectors, ntb->db_count);
				msi = true;
				goto err_msix_enable;
				}

				/* Allocate the necessary number of MSI-x messages */
				rc = pci_alloc_msix(ntb->device, &num_vectors);
				if (rc != 0) {
				amd_ntb_printf(0, "Error allocating msix vectors: %d\n", rc);
				msi = true;
				goto err_msix_enable;
				}

				if (num_vectors < ntb->db_count) {
				amd_ntb_printf(0, "Allocated only %d MSI-X\n", num_vectors);
				msi = true;
				/*
				* Else set ntb->db_count = ntb->msix_vec_count = num_vectors,
				* msi=false and dont release msi
				*/
				}

				err_msix_enable:

				if (msi) {
				free(ntb->msix_vec, M_AMD_NTB);
				ntb->msix_vec = NULL;
				pci_release_msi(ntb->device);
				num_vectors = 1;
				rc = pci_alloc_msi(ntb->device, &num_vectors);
				if (rc != 0) {
				amd_ntb_printf(0, "Error allocating msix vectors: %d\n", rc);
				msi = false;
				intx = true;
				}
				}

				ntb->db_count = ntb->msix_vec_count = num_vectors;

				if (intx) {
				num_vectors = 1;
				ntb->db_count = 1;
				ntb->msix_vec_count = 0;
				}

				amd_ntb_printf(0, "%s: db %d msix %d msi %d intx %d\n",
				__func__, ntb->db_count, ntb->msix_vec_count, (int)msi, (int)intx);

				rc = amd_ntb_setup_isr(ntb, num_vectors, msi, intx);
				if (rc != 0) {
				amd_ntb_printf(0, "Error setting up isr: %d\n", rc);
				amd_ntb_free_msix_vec(ntb);
				}

				return (rc);
				}

				static void
				amd_ntb_deinit_isr(struct amd_ntb_softc *ntb)
				{
				struct amd_ntb_int_info *current_int;
				int i;

				/* Mask all doorbell interrupts */
				ntb->db_mask = ntb->db_valid_mask;
				amd_ntb_reg_write(4, AMD_DBMASK_OFFSET, ntb->db_mask);

				for (i = 0; i < ntb->allocated_interrupts; i++) {
				current_int = &ntb->int_info[i];
				if (current_int->tag != NULL)
				bus_teardown_intr(ntb->device, current_int->res,
				current_int->tag);

				if (current_int->res != NULL)
				bus_release_resource(ntb->device, SYS_RES_IRQ,
				rman_get_rid(current_int->res), current_int->res);
				}

				amd_ntb_free_msix_vec(ntb);
				}

				static enum amd_ntb_conn_type
				amd_ntb_get_topo(struct amd_ntb_softc *ntb)
				{
				uint32_t info;

				info = amd_ntb_reg_read(4, AMD_SIDEINFO_OFFSET);

				if (info & AMD_SIDE_MASK)
				return (NTB_CONN_SEC);

				return (NTB_CONN_PRI);
				}

				static int
				amd_ntb_init_dev(struct amd_ntb_softc *ntb)
				{
				ntb->mw_count = AMD_MW_CNT;
				ntb->spad_count = AMD_SPADS_CNT;
				ntb->db_count = AMD_DB_CNT;
				ntb->db_valid_mask = (1ull << ntb->db_count) - 1;
				mtx_init(&ntb->db_mask_lock, "amd ntb db bits", NULL, MTX_SPIN);

				switch (ntb->conn_type) {
				case NTB_CONN_PRI:
				case NTB_CONN_SEC:
				ntb->spad_count >>= 1;

				if (ntb->conn_type == NTB_CONN_PRI) {
				ntb->self_spad = 0;
				ntb->peer_spad = 0x20;
				} else {
				ntb->self_spad = 0x20;
				ntb->peer_spad = 0;
				}

				callout_init(&ntb->hb_timer, 1);
				callout_reset(&ntb->hb_timer, AMD_LINK_HB_TIMEOUT,
				amd_link_hb, ntb);

				break;

				default:
				amd_ntb_printf(0, "Unsupported AMD NTB topology %d\n",
				ntb->conn_type);
				return (EINVAL);
				}

				ntb->int_mask = AMD_EVENT_INTMASK;
				amd_ntb_reg_write(4, AMD_INTMASK_OFFSET, ntb->int_mask);

				return (0);
				}

				static int
				amd_ntb_init(struct amd_ntb_softc *ntb)
				{
				int rc = 0;

				ntb->conn_type = amd_ntb_get_topo(ntb);
				amd_ntb_printf(0, "AMD NTB Side: %s\n",
				(ntb->conn_type == NTB_CONN_PRI)? "PRIMARY" : "SECONDARY");

				rc = amd_ntb_init_dev(ntb);
				if (rc != 0)
				return (rc);

				rc = amd_ntb_init_isr(ntb);
				if (rc != 0)
				return (rc);

				return (0);
				}

				static void
				print_map_success(struct amd_ntb_softc ntb, struct amd_ntb_pci_bar_info bar,
				const char *kind)
				{
				amd_ntb_printf(0, "Mapped BAR%d v:[%p-%p] p:[%p-%p] (0x%jx bytes) (%s)\n",
				PCI_RID2BAR(bar->pci_resource_id), bar->vbase,
				(char )bar->vbase + bar->size - 1, (void )bar->pbase,
				(void *)(bar->pbase + bar->size - 1), (uintmax_t)bar->size, kind);
				}

				static void
				save_bar_parameters(struct amd_ntb_pci_bar_info *bar)
				{
				bar->pci_bus_tag = rman_get_bustag(bar->pci_resource);
				bar->pci_bus_handle = rman_get_bushandle(bar->pci_resource);
				bar->pbase = rman_get_start(bar->pci_resource);
				bar->size = rman_get_size(bar->pci_resource);
				bar->vbase = rman_get_virtual(bar->pci_resource);
				bar->map_mode = VM_MEMATTR_UNCACHEABLE;
				}

				static int
				map_bar(struct amd_ntb_softc ntb, struct amd_ntb_pci_bar_info bar)
				{
				bar->pci_resource = bus_alloc_resource_any(ntb->device, SYS_RES_MEMORY,
				&bar->pci_resource_id, RF_ACTIVE);
				if (bar->pci_resource == NULL)
				return (ENXIO);

				save_bar_parameters(bar);
				print_map_success(ntb, bar, "mmr");

				return (0);
				}

				static int
				amd_ntb_map_pci_bars(struct amd_ntb_softc *ntb)
				{
				int rc = 0;

				/* NTB Config/Control registers - BAR 0 */
				ntb->bar_info[NTB_CONFIG_BAR].pci_resource_id = PCIR_BAR(0);
				rc = map_bar(ntb, &ntb->bar_info[NTB_CONFIG_BAR]);
				if (rc != 0)
				goto out;

				/* Memory Window 0 BAR - BAR 1*/
				ntb->bar_info[NTB_BAR_1].pci_resource_id = PCIR_BAR(1);
				rc = map_bar(ntb, &ntb->bar_info[NTB_BAR_1]);
				if (rc != 0)
				goto out;
				ntb->bar_info[NTB_BAR_1].xlat_off = AMD_BAR1XLAT_OFFSET;
				ntb->bar_info[NTB_BAR_1].limit_off = AMD_BAR1LMT_OFFSET;

				/* Memory Window 1 BAR - BAR 2&3 */
				ntb->bar_info[NTB_BAR_2].pci_resource_id = PCIR_BAR(2);
				rc = map_bar(ntb, &ntb->bar_info[NTB_BAR_2]);
				if (rc != 0)
				goto out;
				ntb->bar_info[NTB_BAR_2].xlat_off = AMD_BAR23XLAT_OFFSET;
				ntb->bar_info[NTB_BAR_2].limit_off = AMD_BAR23LMT_OFFSET;

				/* Memory Window 2 BAR - BAR 4&5 */
				ntb->bar_info[NTB_BAR_3].pci_resource_id = PCIR_BAR(4);
				rc = map_bar(ntb, &ntb->bar_info[NTB_BAR_3]);
				if (rc != 0)
				goto out;
				ntb->bar_info[NTB_BAR_3].xlat_off = AMD_BAR45XLAT_OFFSET;
				ntb->bar_info[NTB_BAR_3].limit_off = AMD_BAR45LMT_OFFSET;

				out:
				if (rc != 0)
				amd_ntb_printf(0, "unable to allocate pci resource\n");

				return (rc);
				}

				static void
				amd_ntb_unmap_pci_bars(struct amd_ntb_softc *ntb)
				{
				struct amd_ntb_pci_bar_info *bar_info;
				int i;

				for (i = 0; i < NTB_MAX_BARS; i++) {
				bar_info = &ntb->bar_info[i];
				if (bar_info->pci_resource != NULL)
				bus_release_resource(ntb->device, SYS_RES_MEMORY,
				bar_info->pci_resource_id, bar_info->pci_resource);
				}
				}

				static int
				amd_ntb_probe(device_t device)
				{
				const struct pci_device_table *tbl;

				tbl = PCI_MATCH(device, amd_ntb_devs);
				if (tbl == NULL)
				return (ENXIO);

				device_set_desc(device, tbl->descr);

				return (BUS_PROBE_GENERIC);
				}

				static int
				amd_ntb_attach(device_t device)
				{
				struct amd_ntb_softc *ntb = device_get_softc(device);
				int error;

				ntb->device = device;
				impUnsubmitted Done Inline Actions I'd encourage the use of pci_device_table here and the use of PCI_PNP_INFO instead below. That's not as important as the PNP_INFO macro though. imp: I'd encourage the use of pci_device_table here and the use of PCI_PNP_INFO instead below.
				rajeshaspAuthorUnsubmitted Done Inline Actions Changed the PCI IDS to pci_device_table format and used PCI_PNP_INFO rather than MODULE_PNP_INFO. rajeshasp: Changed the PCI IDS to pci_device_table format and used PCI_PNP_INFO rather than…

				/* Enable PCI bus mastering for "device" */
				pci_enable_busmaster(ntb->device);

				error = amd_ntb_map_pci_bars(ntb);
				if (error)
				goto out;

				error = amd_ntb_init(ntb);
				if (error)
				goto out;

				amd_init_side_info(ntb);

				amd_ntb_spad_clear(ntb);

				amd_ntb_sysctl_init(ntb);

				/* Attach children to this controller */
				error = ntb_register_device(device);

				out:
				if (error)
				amd_ntb_detach(device);

				return (error);
				}

				static int
				amd_ntb_detach(device_t device)
				{
				struct amd_ntb_softc *ntb = device_get_softc(device);

				ntb_unregister_device(device);
				amd_deinit_side_info(ntb);
				callout_drain(&ntb->hb_timer);
				amd_ntb_deinit_isr(ntb);
				mtx_destroy(&ntb->db_mask_lock);
				pci_disable_busmaster(ntb->device);
				amd_ntb_unmap_pci_bars(ntb);

				return (0);
				}

				static device_method_t ntb_amd_methods[] = {
				/* Device interface */
				DEVMETHOD(device_probe, amd_ntb_probe),
				DEVMETHOD(device_attach, amd_ntb_attach),
				DEVMETHOD(device_detach, amd_ntb_detach),

				/* Bus interface */
				DEVMETHOD(bus_child_location_str, ntb_child_location_str),
				DEVMETHOD(bus_print_child, ntb_print_child),

				/* NTB interface */
				DEVMETHOD(ntb_port_number, amd_ntb_port_number),
				DEVMETHOD(ntb_peer_port_count, amd_ntb_peer_port_count),
				DEVMETHOD(ntb_peer_port_number, amd_ntb_peer_port_number),
				DEVMETHOD(ntb_peer_port_idx, amd_ntb_peer_port_idx),
				DEVMETHOD(ntb_link_is_up, amd_ntb_link_is_up),
				DEVMETHOD(ntb_link_enable, amd_ntb_link_enable),
				DEVMETHOD(ntb_link_disable, amd_ntb_link_disable),
				DEVMETHOD(ntb_mw_count, amd_ntb_mw_count),
				DEVMETHOD(ntb_mw_get_range, amd_ntb_mw_get_range),
				DEVMETHOD(ntb_mw_set_trans, amd_ntb_mw_set_trans),
				DEVMETHOD(ntb_mw_clear_trans, amd_ntb_mw_clear_trans),
				DEVMETHOD(ntb_mw_set_wc, amd_ntb_mw_set_wc),
				DEVMETHOD(ntb_mw_get_wc, amd_ntb_mw_get_wc),
				DEVMETHOD(ntb_db_valid_mask, amd_ntb_db_valid_mask),
				DEVMETHOD(ntb_db_vector_count, amd_ntb_db_vector_count),
				DEVMETHOD(ntb_db_vector_mask, amd_ntb_db_vector_mask),
				DEVMETHOD(ntb_db_read, amd_ntb_db_read),
				DEVMETHOD(ntb_db_clear, amd_ntb_db_clear),
				DEVMETHOD(ntb_db_set_mask, amd_ntb_db_set_mask),
				DEVMETHOD(ntb_db_clear_mask, amd_ntb_db_clear_mask),
				DEVMETHOD(ntb_peer_db_set, amd_ntb_peer_db_set),
				DEVMETHOD(ntb_spad_count, amd_ntb_spad_count),
				DEVMETHOD(ntb_spad_read, amd_ntb_spad_read),
				DEVMETHOD(ntb_spad_write, amd_ntb_spad_write),
				DEVMETHOD(ntb_peer_spad_read, amd_ntb_peer_spad_read),
				DEVMETHOD(ntb_peer_spad_write, amd_ntb_peer_spad_write),
				DEVMETHOD_END
				};

				static DEFINE_CLASS_0(ntb_hw, ntb_amd_driver, ntb_amd_methods,
				sizeof(struct amd_ntb_softc));
				DRIVER_MODULE(ntb_hw_amd, pci, ntb_amd_driver, ntb_hw_devclass, NULL, NULL);
				MODULE_DEPEND(ntb_hw_amd, ntb, 1, 1, 1);
				MODULE_VERSION(ntb_hw_amd, 1);
				PCI_PNP_INFO(amd_ntb_devs);
				cemUnsubmitted Done Inline Actions I'd encourage adding a `MODULE_PNP_INFO()`, like the Intel NTB driver has. cem: I'd encourage adding a `MODULE_PNP_INFO()`, like the Intel NTB driver has.
				rajeshaspAuthorUnsubmitted Done Inline Actions I see this macro register device information. But how useful that will be with the system overall? It may be simple question, but just trying to understand. rajeshasp: I see this macro register device information. But how useful that will be with the system…
				impUnsubmitted Not Done Inline Actions It's used by devmatch to automatically load modules based on plug and play information. imp: It's used by devmatch to automatically load modules based on plug and play information.