fix(v4): a controller's drive slots are allocated once, never replaced (#178)
sk_v4_idle allocated a controller's slot array again whenever max_slots + 1 was more than it held -- the old array dropped, neither copied nor freed -- while the identity code holds pointers into it (g_offered, g_spare, g_pending_real, g_real_drive, g_zuse_dev). Its two slot loops then ran to max_slots whatever the array held. sk_v4_slots_get() (kernel/src/v4/sk_v4_slots.c): allocated once, zeroed, at the size first asked for; never replaced; the slots that may be used are those it has. sk_v4_idle uses it, and its loops stop at the array's end. kernel/tests/test_sk_v4_slots.c, kmalloc_aligned faked, seen to fail first against the allocation as it was (4 failures: replaced when more were asked for, 17 said usable where 9 existed, a held pointer left behind, an allocation for 0); now 6 checks, 0 failures. Built for amd64, aarch64, riscv64; latent today (max_slots does not grow), so the boots show no change. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017RzZgrf7V7Zk2iczBXDRbm
This commit is contained in:
+2
-2
@@ -730,8 +730,8 @@ V4_ENGINE_SRCS := $(addprefix v4/src/,node.c exec.c stack.c iword.c heat.c guard
|
||||
V4_ROM_C := $(BUILD_DIR)/v4_rom_64.c
|
||||
V4_MAP_C := $(BUILD_DIR)/v4_map_64.c
|
||||
|
||||
LOADER_EXTRA_SRCS += $(KERNEL_SRC)/v4/sk_v4.c $(KERNEL_SRC)/v4/sk_v4_identity.c $(KERNEL_SRC)/v4/sk_v4_marker.c $(KERNEL_SRC)/v4/sk_v4_grow.c $(KERNEL_SRC)/v4/sk_v4_mint.c $(KERNEL_SRC)/v4/sk_v4_stop.c
|
||||
KERNEL_EXTRA_SRCS += $(KERNEL_SRC)/v4/sk_v4.c $(KERNEL_SRC)/v4/sk_v4_identity.c $(KERNEL_SRC)/v4/sk_v4_marker.c $(KERNEL_SRC)/v4/sk_v4_grow.c $(KERNEL_SRC)/v4/sk_v4_mint.c $(KERNEL_SRC)/v4/sk_v4_stop.c
|
||||
LOADER_EXTRA_SRCS += $(KERNEL_SRC)/v4/sk_v4.c $(KERNEL_SRC)/v4/sk_v4_identity.c $(KERNEL_SRC)/v4/sk_v4_marker.c $(KERNEL_SRC)/v4/sk_v4_grow.c $(KERNEL_SRC)/v4/sk_v4_mint.c $(KERNEL_SRC)/v4/sk_v4_stop.c $(KERNEL_SRC)/v4/sk_v4_slots.c
|
||||
KERNEL_EXTRA_SRCS += $(KERNEL_SRC)/v4/sk_v4.c $(KERNEL_SRC)/v4/sk_v4_identity.c $(KERNEL_SRC)/v4/sk_v4_marker.c $(KERNEL_SRC)/v4/sk_v4_grow.c $(KERNEL_SRC)/v4/sk_v4_mint.c $(KERNEL_SRC)/v4/sk_v4_stop.c $(KERNEL_SRC)/v4/sk_v4_slots.c
|
||||
# v4/system/boot.c is the boot the hosted v4 system runs too: the ROM
|
||||
# assembles the system, then the capsules from this kernel's own capsule
|
||||
# directory, then the prompt.
|
||||
|
||||
@@ -0,0 +1,17 @@
|
||||
/* SPDX-License-Identifier: LicenseRef-Starship-1.0
|
||||
* Copyright (c) 2023-2026 Robert A. James
|
||||
*/
|
||||
/* sk_v4_slots.h -- an xHCI controller's drive slots, for sk_v4_identity.c (#178): the array the
|
||||
* attached drives live in, which the identity code holds pointers into (the offered stick, the
|
||||
* spare, the real Zuse's, Zuse's own device). */
|
||||
#ifndef STARKERNEL_V4_SK_V4_SLOTS_H
|
||||
#define STARKERNEL_V4_SK_V4_SLOTS_H
|
||||
|
||||
#include <stddef.h>
|
||||
#include <stdint.h>
|
||||
|
||||
/* The slot array in *slots, `elem` bytes a slot, *count of them: allocated (zeroed) when there is
|
||||
* none, with `want` slots. Returns how many slots may be used (0: none could be allocated). */
|
||||
uint32_t sk_v4_slots_get(void **slots, uint32_t *count, uint32_t want, size_t elem);
|
||||
|
||||
#endif
|
||||
@@ -35,6 +35,7 @@
|
||||
* ACL-ZUSE-BOOT.
|
||||
*/
|
||||
#include "starkernel/v4/sk_v4_identity.h"
|
||||
#include "starkernel/v4/sk_v4_slots.h"
|
||||
#include "starkernel/console.h"
|
||||
#include "starkernel/arch.h"
|
||||
#include "starkernel/apic.h"
|
||||
@@ -510,15 +511,14 @@ static void sk_v4_idle(void)
|
||||
for (xc = 0; xc < xhci_dev_count() && xc < XHCI_MAX_CONTROLLERS; xc++) {
|
||||
xhci_dev_t *xdev = xhci_dev_at(xc);
|
||||
if (!xdev) continue;
|
||||
if (!g_usb[xc].slots || g_usb[xc].count < xdev->max_slots + 1) {
|
||||
size_t bytes = (size_t)(xdev->max_slots + 1) * sizeof(sk_v4_drive_t);
|
||||
sk_v4_drive_t *fresh = (sk_v4_drive_t *)kmalloc_aligned(bytes, 64);
|
||||
if (!fresh) continue;
|
||||
memset(fresh, 0, bytes);
|
||||
g_usb[xc].slots = fresh;
|
||||
g_usb[xc].count = xdev->max_slots + 1;
|
||||
/* allocated once and never replaced: the identity code holds pointers into it (#178) */
|
||||
{
|
||||
void *slots = g_usb[xc].slots;
|
||||
uint32_t usable = sk_v4_slots_get(&slots, &g_usb[xc].count, xdev->max_slots + 1, sizeof(sk_v4_drive_t));
|
||||
g_usb[xc].slots = (sk_v4_drive_t *)slots;
|
||||
if (usable == 0) continue;
|
||||
}
|
||||
for (slot = 1; slot <= xdev->max_slots; slot++) {
|
||||
for (slot = 1; slot <= xdev->max_slots && slot < g_usb[xc].count; slot++) {
|
||||
xhci_msc_slot_t *ms = xhci_msc_slot_for(xdev, slot);
|
||||
if (!ms || !ms->bot_msc_attach_pending) continue;
|
||||
ms->bot_msc_attach_pending = 0;
|
||||
@@ -526,7 +526,7 @@ static void sk_v4_idle(void)
|
||||
sk_v4_drive_arrived(&g_usb[xc].slots[slot]))
|
||||
ms->bot_msc_attached = 1;
|
||||
}
|
||||
for (slot = 1; slot <= xdev->max_slots; slot++) {
|
||||
for (slot = 1; slot <= xdev->max_slots && slot < g_usb[xc].count; slot++) {
|
||||
xhci_msc_slot_t *ms = xhci_msc_slot_for(xdev, slot);
|
||||
if (!ms || !ms->bot_msc_detach_pending) continue;
|
||||
ms->bot_msc_detach_pending = 0;
|
||||
|
||||
@@ -0,0 +1,23 @@
|
||||
/* SPDX-License-Identifier: LicenseRef-Starship-1.0
|
||||
* Copyright (c) 2023-2026 Robert A. James
|
||||
*/
|
||||
/* sk_v4_slots.c -- see sk_v4_slots.h. */
|
||||
#include "starkernel/v4/sk_v4_slots.h"
|
||||
#include "starkernel/kmalloc.h"
|
||||
#include <string.h>
|
||||
|
||||
/* sk_v4_idle replaced the array whenever a controller's max_slots + 1 was more than it held: the
|
||||
* old one dropped, neither copied nor freed, while the identity code held pointers into it (#178).
|
||||
* Now it is allocated once, at the size first asked for, and never replaced; the slots that may be
|
||||
* used are the ones it has. */
|
||||
uint32_t sk_v4_slots_get(void **slots, uint32_t *count, uint32_t want, size_t elem)
|
||||
{
|
||||
if (!*slots && want) {
|
||||
void *fresh = kmalloc_aligned(want * elem, 64);
|
||||
if (!fresh) return 0;
|
||||
memset(fresh, 0, want * elem);
|
||||
*slots = fresh;
|
||||
*count = want;
|
||||
}
|
||||
return *slots ? *count : 0;
|
||||
}
|
||||
@@ -19,7 +19,8 @@ CC ?= cc
|
||||
CFLAGS := -std=c99 -Wall -Wextra -Werror -O2 -g -I$(HERE) -I$(ROOT)/kernel/include \
|
||||
-I$(ROOT)/kernel/include/starkernel -I$(ROOT)/v3/include
|
||||
|
||||
TESTS := test_gpt test_usb_config test_sk_v4_stop
|
||||
TESTS := test_gpt test_usb_config test_sk_v4_stop test_sk_v4_slots
|
||||
SRC_test_sk_v4_slots := $(ROOT)/kernel/src/v4/sk_v4_slots.c
|
||||
SRC_test_sk_v4_stop := $(ROOT)/kernel/src/v4/sk_v4_stop.c
|
||||
SRC_test_gpt := $(ROOT)/kernel/src/gpt.c
|
||||
SRC_test_usb_config := $(ROOT)/kernel/src/usb/usb_config.c
|
||||
|
||||
@@ -0,0 +1,47 @@
|
||||
/* SPDX-License-Identifier: LicenseRef-Starship-1.0
|
||||
* Copyright (c) 2023-2026 Robert A. James
|
||||
*/
|
||||
/* test_sk_v4_slots.c -- kernel/src/v4/sk_v4_slots.c on the host (#178): a controller's drive slots
|
||||
* are allocated once and never replaced, so the pointers the identity code keeps into them (the
|
||||
* offered stick, the spare, Zuse's device) stay good. kmalloc_aligned is faked.
|
||||
*/
|
||||
#define _POSIX_C_SOURCE 200809L
|
||||
#include "harness.h"
|
||||
#include "starkernel/v4/sk_v4_slots.h"
|
||||
#include <stdlib.h>
|
||||
#include <string.h>
|
||||
|
||||
static unsigned allocs;
|
||||
void *kmalloc_aligned(size_t size, size_t align) { void *p = 0; allocs++; return posix_memalign(&p, align, size) ? 0 : p; }
|
||||
|
||||
typedef struct { int attached; char name[16]; } drive; /* a slot, as sk_v4_drive_t is to the identity code */
|
||||
|
||||
int main(void)
|
||||
{
|
||||
void *slots = 0;
|
||||
uint32_t count = 0, n;
|
||||
drive *d, *held;
|
||||
|
||||
n = sk_v4_slots_get(&slots, &count, 9u, sizeof(drive));
|
||||
d = (drive *)slots;
|
||||
CHECK(n == 9u && d && allocs == 1u && d[3].attached == 0, "first: 9 zeroed slots, one allocation (n %u)", n);
|
||||
d[3].attached = 1; strcpy(d[3].name, "Zuse");
|
||||
held = &d[3]; /* as g_zuse_dev, g_offered, g_spare hold one */
|
||||
|
||||
n = sk_v4_slots_get(&slots, &count, 9u, sizeof(drive));
|
||||
CHECK(slots == (void *)d && n == 9u && allocs == 1u, "again, the same size: the same array, no allocation");
|
||||
|
||||
n = sk_v4_slots_get(&slots, &count, 17u, sizeof(drive));
|
||||
CHECK(slots == (void *)d && allocs == 1u, "#178: asked for more: the same array, not replaced (allocations %u)", allocs);
|
||||
CHECK(n == 9u, "#178: and only the 9 slots it has may be used, not 17 (n %u)", n);
|
||||
CHECK(held->attached == 1 && strcmp(held->name, "Zuse") == 0 && held == &((drive *)slots)[3],
|
||||
"#178: a pointer held into a slot is still the slot, with the drive in it");
|
||||
|
||||
{
|
||||
void *none = 0; uint32_t c = 0;
|
||||
unsigned before = allocs;
|
||||
CHECK(sk_v4_slots_get(&none, &c, 0u, sizeof(drive)) == 0u && none == 0 && allocs == before,
|
||||
"0 slots wanted: none to use, nothing allocated");
|
||||
}
|
||||
return DONE();
|
||||
}
|
||||
Binary file not shown.
Reference in New Issue
Block a user