Register and memory access device parameter is not specific enough

jamiebird-arm commented 2 years ago

Register access callbacks note:

//! The _device_ parameter indicates the address of the AP to access. It can also be set to
//! @ref SDM_DefaultDevice, and the debugger will use a default AP. For ADIv5 systems, the
//! AP address is an APSEL value in the range 0-255. For ADIv6 systems, the AP address is an
//! APB address whose width depends on the target implementation. Nested ADIv6 APs are not
//! supported directly.

This does not allow register access for a CoreSight generic component, and if the docs where updated to allow this I don't think the device parameter would be specific enough. For example, on a ADIv6 system, device = 0x100 could mean:

CoreSight component in DP memory space @ 0x100
AP @ base address 0x100
CoreSight component under any number of APs @ 0x100

On a ADIv5 system, device = 0x100 could mean:

AP index 256 (not architecturally allowed but example still stands)
CoreSight component under any number of APs @ 0x100

And memory access callbacks note:

//! The _device_ parameter indicates the a debug-architecture-defined address for the interface
//! to the memory system. For the Arm ADI architecture, this is the address of the MEM-AP to use.
//! It can also be set to #SDM_DefaultDevice, and the debugger will use the default memory interface
//! device identified in SDMOpenParmeters. If no default memory interface device is indicated, then
//! use of #SDM_DefaultDevice will return an error.
//!
//! The _address_ parameter is always the address to access within the memory space controlled
//! by the selected device. For MEM-APs, this is obvious. When #SDM_DefaultDevice is used and the
//! default device type is #SDM_ArmADI_CoreSight_Component, then the _address_ parameter becomes an
//! offset relative to the base address of the CoreSight component's 4 kB memory region.

While this definition is more flexible regarding CoreSight components, it seems to limit them to only being accessible if they are the SDM_DefaultDevice, which is quite restrictive.

jamiebird-arm commented 2 years ago

With my comment (https://github.com/ARM-software/sdm-api/pull/16#discussion_r801141792) on the manifest.xml schema regard multiple mailbox definitions, would it make sense to limit the devices the SDm can access to those defined in the manifest? That way the SDM callbacks can directly reference devices, maybe via a unique ID.

Example manifest.xml

<!-- Example: Arm SDC-600 by AP IDR -->
<mailbox sdm_id="default" type="apv1" idr="0x04762000" info="Arm SDC-600"/>

<!-- Example: NXP's DM-AP by AP IDR -->
<mailbox sdm_id="1" type="apv1" idr="0x002a0000" info="NXP LPC55xx DM-AP"/>

<!-- Example: Nordic's CTRL-AP by AP IDR -->
<mailbox sdm_id="2" type="apv1" idr="0x02880000" info="Nordic nRF CTRL-AP"/>

<!-- Example: APv1 with explicit AP index -->
<mailbox sdm_id="3" type="apv1" index="3" info="SDC-600"/>

<!-- Example: APv2 with explit address -->
<mailbox sdm_id="4" type="apv2" base="0x4000" info="Example APv2"/>

Then the device parameter could simply be:

* @param[in] device Unique SDM device ID from SDM manifest, or #SDM_DefaultDevice.

I supposed this solution would come with the downfall that the SDM now has a hard link to the manifest.xml, but only if multiple devices/mailboxes are defined. If only a single default device is used then the SDM could be reused as before.

Thoughts?

flit commented 2 years ago

ARM-software / sdm-api

Register and memory access device parameter is not specific enough #18