Re: [PATCH net-next 0/9] devlink: Add support for region access

From: Alex Vesker <valex@mellanox.com>
To: Andrew Lunn <andrew@lunn.ch>
Cc: "David S. Miller" <davem@davemloft.net>, <netdev@vger.kernel.org>,
	"Tariq Toukan" <tariqt@mellanox.com>,
	Jiri Pirko <jiri@mellanox.com>
Subject: Re: [PATCH net-next 0/9] devlink: Add support for region access
Date: Thu, 29 Mar 2018 21:59:53 +0300	[thread overview]
Message-ID: <962b56c1-d471-97ec-e8e9-18252e809dfe@mellanox.com> (raw)
In-Reply-To: <20180329171359.GA12150@lunn.ch>

On 3/29/2018 8:13 PM, Andrew Lunn wrote:
> On Thu, Mar 29, 2018 at 07:07:43PM +0300, Alex Vesker wrote:
>> This is a proposal which will allow access to driver defined address
>> regions using devlink. Each device can create its supported address
>> regions and register them. A device which exposes a region will allow
>> access to it using devlink.
>>
>> The suggested implementation will allow exposing regions to the user,
>> reading and dumping snapshots taken from different regions.
>> A snapshot represents a memory image of a region taken by the driver.
>>
>> If a device collects a snapshot of an address region it can be later
>> exposed using devlink region read or dump commands.
>> This functionality allows for future analyses on the snapshots to be
>> done.
> Hi Alex
>
> So the device is in change of making a snapshot? A user cannot
> initiate it?
Hi,
Correct, currently the user cannot initiate saving a snapshot but
as I said in the cover letter, planned support is for dumping "live" 
regions.

> Seems like if i'm trying to debug something, i want to take a snapshot
> in the good state, issue the command which breaks things, and then
> take another snapshot. Looking at the diff then gives me an idea what
> happened.
>
>> Show all of the exposed regions with region sizes:
>> $ devlink region show
>> pci/0000:00:05.0/cr-space: size 1048576 snapshot [1 2]
> So you have 2Mbytes of snapshot data. Is this held in the device, or
> kernel memory?
This is allocated in devlink, the maximum number of snapshots is set by 
the driver.

>> Dump a snapshot:
>> $ devlink region dump pci/0000:00:05.0/fw-health snapshot 1
>> 0000000000000000 0014 95dc 0014 9514 0035 1670 0034 db30
>> 0000000000000010 0000 0000 ffff ff04 0029 8c00 0028 8cc8
>> 0000000000000020 0016 0bb8 0016 1720 0000 0000 c00f 3ffc
>> 0000000000000030 bada cce5 bada cce5 bada cce5 bada cce5
>>
>> Read a specific part of a snapshot:
>> $ devlink region read pci/0000:00:05.0/fw-health snapshot 1 address 0
>> 	length 16
>> 0000000000000000 0014 95dc 0014 9514 0035 1670 0034 db30
> Why a separate command? It seems to be just a subset of dump.

This is useful when debugging values on specific addresses, this also
brings the API one step closer for a read and write API.

>
>      Andrew