Skip to content

[vioscsi] Extend VioScsiReadRegistryParameter() #1216

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

[vioscsi] Extend VioScsiReadRegistryParameter() #1216

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
245 changes: 229 additions & 16 deletions vioscsi/vioscsi.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -220,35 +220,247 @@ USHORT CopyBufferToAnsiString(void *_pDest, const void *_pSrc, const char delimi

BOOLEAN VioScsiReadRegistryParameter(IN PVOID DeviceExtension, IN PUCHAR ValueName, IN LONG offset)
{
BOOLEAN Ret = FALSE;
ULONG Len = sizeof(ULONG);
UCHAR *pBuf = NULL;
PADAPTER_EXTENSION adaptExt;
BOOLEAN bReadResult = FALSE;
BOOLEAN bUseAltPerHbaRegRead = FALSE;
ULONG pBufferLength = sizeof(ULONG);
UCHAR *pBuffer = NULL;
PADAPTER_EXTENSION adaptExt = (PADAPTER_EXTENSION)DeviceExtension;
ULONG spgspn_rc, i, j;
STOR_ADDRESS HwAddress = {0};
PSTOR_ADDRESS pHwAddress = &HwAddress;
CHAR valname_as_str[64] = {0};
CHAR hba_id_as_str[4] = {0};
USHORT shAdapterId = (USHORT)adaptExt->slot_number - 1;
#if !defined(RUN_UNCHECKED)
ULONG value_as_ulong; // Used in TRACING.
#endif

adaptExt = (PADAPTER_EXTENSION)DeviceExtension;
pBuf = StorPortAllocateRegistryBuffer(DeviceExtension, &Len);
if (pBuf == NULL)
/* Get a clean buffer to store the registry value... */
pBuffer = StorPortAllocateRegistryBuffer(DeviceExtension, &pBufferLength);
if (pBuffer == NULL)
{
RhelDbgPrint(TRACE_LEVEL_FATAL, "StorPortAllocateRegistryBuffer failed to allocate buffer\n");
#if !defined(RUN_UNCHECKED)
RhelDbgPrint(TRACE_LEVEL_WARNING, " StorPortAllocateRegistryBuffer failed to allocate buffer\n");
#endif
return FALSE;
}
memset(pBuffer, 0, sizeof(ULONG));

memset(pBuf, 0, sizeof(ULONG));
/* Check if we can get a System PortNumber to access the \Parameters\Device(d) subkey to get a per HBA value.
* FIXME NOTE
*
* Regarding StorPortGetSystemPortNumber():
*
* StorPort always reports STOR_STATUS_INVALID_DEVICE_STATE and does not update pHwAddress->Port.
* Calls to StorPortRegistryRead() and StorPortRegistryWrite() only read or write to \Parameters\Device-1,
* which appears to be an uninitialized value. Therefore, the alternate per HBA read technique will always be used.
*
* Various initialisation syntaxes were attempted, including partial and fully initialized STOR_ADDRESS and
* STOR_ADDR_BTL8 structs and pointers. Attempts to initialize most of the deprecated HW_INITIALIZATION_DATA
* and PORT_CONFIGURATION_INFORMATION members were also made, but of no effect with regard to this function.
* Using DeviceExtension or the adaptExt pointer as the first parameter to the function had no effect.
* Attempts to set the InitiatorBusId were successful, but of no effect with regard to this function.
* Also attempted BusType = BusTypeScsi (rather than BusTypeSas per inf default) - in both the inf and using
* StorPortSetAdapterBusType() too. Also tried many other BusTypes via StorPortSetAdapterBusType() mechanics.
* Maybe something in WMI or VPD processing...? Do we need PortAttributes.PortState = HBA_PORTSTATE_ONLINE and
* PortAttributes.PortType = HBA_PORTTYPE_SASDEVICE to be set...? Should we be initializing adaptExt->wwn,
* adaptExt->port_wwn or adaptExt->port_idx...? The wMI routines are not using the InstanceIndex and InstanceCount
* parameters to cycle through HBAs. Maybe they should...
*
* Difficult to determine what is wrong here...
* ¯\_(ツ)_/¯
*
Copy link
Contributor Author

@benyamin-codez benyamin-codez Mar 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My present thoughts on this issue with Parameters\Device(d) not initialising:
Being a BusTypeSas adapter, I suspect we may not be setting adaptExt->port_idx early enough.
I'll try to test this in the next few days...
...but if anyone has any insights, please do let me know ASAP before I burn more time on this.

* FIXME NOTE END
*/
pHwAddress->Type = STOR_ADDRESS_TYPE_BTL8;
pHwAddress->AddressLength = STOR_ADDR_BTL8_ADDRESS_LENGTH;
#if !defined(RUN_UNCHECKED)
RhelDbgPrint(TRACE_REGISTRY,
" Checking whether the HBA system port number and HBA specific registry are available for reading... "
"\n");
#endif
spgspn_rc = StorPortGetSystemPortNumber(DeviceExtension, pHwAddress);
if (spgspn_rc == STOR_STATUS_INVALID_DEVICE_STATE)
{
#if !defined(RUN_UNCHECKED)
RhelDbgPrint(TRACE_REGISTRY,
" WARNING : !!!...HBA Port not ready yet...!!! Returns : 0x%x (STOR_STATUS_INVALID_DEVICE_STATE) "
"\n",
spgspn_rc);
#endif
/*
* When we are unable to get a valid system PortNumber, we need to
* use an alternate per HBA registry read technique. The technique
* implemented here uses per HBA registry value names based on the
* Storport provided slot_number minus one, padded to hundreds,
* e.g. \Parameters\Device\Valuename_123.
*
* This permits up to 999 HBAs. That ought to be enough... c( O.O )ɔ
Copy link
Contributor Author

@benyamin-codez benyamin-codez Mar 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vrozenfe @YanVugenfirer
c( O.O )ɔ <-- This is me looking at you guys in astonishment...
... after saying catering for 999 (incorrectly - it should be 1,000 btw) HBAs in a system should be enough...!
I decided to use a generous value to demonstrate the ASCII conversion below.
If you want to provide a more realistic number it could simplify that part.
You will notice the division is on Base10 boundaries (units, tens, hundreds).
Therefore, the maximums would be 10, 100 and 1,000 HBAs.
I suspect we only need units and tens, catering for up to 100 HBAs, but let me know what you think.

*/
bUseAltPerHbaRegRead = TRUE;
#if !defined(RUN_UNCHECKED)
RhelDbgPrint(TRACE_REGISTRY,
" Using alternate per HBA registry read technique [\\Parameters\\Device\\Value_(ddd)]. \n");
#endif

Ret = StorPortRegistryRead(DeviceExtension, ValueName, 1, MINIPORT_REG_DWORD, pBuf, &Len);
/* Grab the first 60 characters of the target Registry Value.
* Value name limit is 16,383 characters, so this is important.
* We leave the last 4 characters for the hba_id_as_str values.
* NULL terminator wraps things up. Also used in TRACING.
*/
CopyBufferToAnsiString(&valname_as_str, ValueName, '\0', 60);
CopyBufferToAnsiString(&hba_id_as_str, &shAdapterId, '\0', 4);

if ((Ret == FALSE) || (Len == 0))
/* Convert from integer to padded ASCII numbers. */
if (shAdapterId / 100)
{
j = 0;
hba_id_as_str[j] = (UCHAR)(shAdapterId / 100) + 48;
}
else
{
hba_id_as_str[0] = 48;
if (shAdapterId / 10)
{
j = 1;
hba_id_as_str[j] = (UCHAR)(shAdapterId / 10) + 48;
}
else
{
hba_id_as_str[1] = 48;
j = 2;
hba_id_as_str[j] = (UCHAR)shAdapterId + 48;
}
}
if ((j < 1) && (shAdapterId / 10))
{
j = 1;
hba_id_as_str[j] = (UCHAR)(((shAdapterId - ((shAdapterId / 100) * 100)) / 10) + 48);
}
else if ((j < 2) && (shAdapterId > 9))
{
j = 2;
hba_id_as_str[j] = (UCHAR)((shAdapterId - ((shAdapterId / 10) * 10)) + 48);
}
else
{
j = 1;
hba_id_as_str[j] = 48;
}
if ((j < 2) && (shAdapterId > 0))
{
j = 2;
hba_id_as_str[j] = (UCHAR)((shAdapterId - ((shAdapterId / 10) * 10)) + 48);
}
else if (j < 2)
{
j = 2;
hba_id_as_str[j] = 48;
}
/* NULL-terminate the string. */
hba_id_as_str[3] = '\0';
/* Skip the exisitng ValueName. */
for (i = 0; valname_as_str[i] != '\0'; ++i)
{
}
/* Append an underscore. */
valname_as_str[i] = '\x5F';
/* Append the padded HBA ID and NULL terminator. */
for (j = 0; j < 4; ++j)
{
valname_as_str[i + j + 1] = hba_id_as_str[j];
}

PUCHAR ValueNamePerHba = (UCHAR *)&valname_as_str;
bReadResult = StorPortRegistryRead(DeviceExtension,
ValueNamePerHba,
1,
MINIPORT_REG_DWORD,
pBuffer,
&pBufferLength);
}
else
{
#if !defined(RUN_UNCHECKED)
RhelDbgPrint(TRACE_REGISTRY, " HBA Port : %u | Returns : 0x%x \n", pHwAddress->Port, spgspn_rc);
RhelDbgPrint(TRACE_REGISTRY, " Using StorPort-based per HBA registry read [\\Parameters\\Device(d)]. \n");
#endif
/* FIXME : THIS DOES NOT WORK. IT WILL NOT READ \Parameters\Device(d) subkeys...
* NOTE : Only MINIPORT_REG_DWORD values are supported.
*/
bReadResult = StorPortRegistryRead(DeviceExtension, ValueName, 0, MINIPORT_REG_DWORD, pBuffer, &pBufferLength);
#if !defined(RUN_UNCHECKED)
/* Grab the first 64 characters of the target Registry Value.
* Value name limit is 16,383 characters, so this is important.
* NULL terminator wraps things up. Used in TRACING.
*/
CopyBufferToAnsiString(&valname_as_str, ValueName, '\0', 64);
#endif
}

if ((bReadResult == FALSE) || (pBufferLength == 0))
{
#if !defined(RUN_UNCHECKED)
RhelDbgPrint(TRACE_REGISTRY,
" StorPortRegistryRead was unable to find a per HBA value %s. Attempting to find a global "
"value... \n",
(bUseAltPerHbaRegRead) ? "using \\Parameters\\Device\\Value_(ddd) value names"
: "at the \\Parameters\\Device(d) subkey");
#endif
bReadResult = FALSE;
pBufferLength = sizeof(ULONG);
memset(pBuffer, 0, sizeof(ULONG));

/* Do a "Global" read of the Parameters\Device subkey...
* NOTE : Only MINIPORT_REG_DWORD values are supported.
*/
bReadResult = StorPortRegistryRead(DeviceExtension, ValueName, 1, MINIPORT_REG_DWORD, pBuffer, &pBufferLength);
#if !defined(RUN_UNCHECKED)
/* Grab the first 64 characters of the target Registry Value.
* Value name limit is 16,383 characters, so this is important.
* NULL terminator wraps things up. Used in TRACING.
*/
CopyBufferToAnsiString(&valname_as_str, ValueName, '\0', 64);
#endif
}
#if !defined(RUN_UNCHECKED)
/* Give me the DWORD Registry Value as a ULONG from the pointer.
* Used in TRACING.
*/
memcpy(&value_as_ulong, pBuffer, sizeof(ULONG));
#endif

if ((bReadResult == FALSE) || (pBufferLength == 0))
{
RhelDbgPrint(TRACE_LEVEL_FATAL, "StorPortRegistryRead returned 0x%x, Len = %d\n", Ret, Len);
StorPortFreeRegistryBuffer(DeviceExtension, pBuf);
#if !defined(RUN_UNCHECKED)
RhelDbgPrint(TRACE_REGISTRY,
" StorPortRegistryRead of %s returned NOT FOUND or EMPTY, pBufferLength = %d, Possible "
"pBufferLength Hint = 0x%x (%lu) \n",
valname_as_str,
pBufferLength,
value_as_ulong,
value_as_ulong);
#endif
StorPortFreeRegistryBuffer(DeviceExtension, pBuffer);
return FALSE;
}
else
{
#if !defined(RUN_UNCHECKED)
RhelDbgPrint(TRACE_REGISTRY,
" StorPortRegistryRead of %s returned SUCCESS, pBufferLength = %d, Value = 0x%x (%lu) \n",
valname_as_str,
pBufferLength,
value_as_ulong,
value_as_ulong);
#endif

StorPortCopyMemory((PVOID)((UINT_PTR)adaptExt + offset), (PVOID)pBuf, sizeof(ULONG));
StorPortCopyMemory((PVOID)((UINT_PTR)adaptExt + offset), (PVOID)pBuffer, sizeof(ULONG));

StorPortFreeRegistryBuffer(DeviceExtension, pBuf);
StorPortFreeRegistryBuffer(DeviceExtension, pBuffer);

return TRUE;
return TRUE;
}
}

ULONG
Expand Down Expand Up @@ -361,6 +573,7 @@ VioScsiFindAdapter(IN PVOID DeviceExtension,

adaptExt->dump_mode = IsCrashDumpMode;
adaptExt->hba_id = HBA_ID;

ConfigInfo->Master = TRUE;
ConfigInfo->ScatterGather = TRUE;
ConfigInfo->DmaWidth = Width32Bits;
Expand Down