CE copy fix deadlock, libu fr/fw fix missing volume_close () on failure, XHCI use ring-specific pending flags, usbdrv Fix spin_unlock/free ordering

ce/interp.c

@@ -25,6 +25,7 @@
 #include <tcursor.h>
 #include <terminal.h>
 #include <tscreen.h>
+#include <write_file.h>
 
 static bool run = true;
 
@@ -207,91 +208,93 @@ static void cat (struct context* context, char** file_paths, size_t files_count)
 }
 
 static void copy (struct context* context, const char* path_a, const char* path_b) {
-  char volume[VOLUME_MAX];
-  const char* path;
+  char volume_a[VOLUME_MAX], volume_b[VOLUME_MAX];
+  const char *path_a1, *path_b1;
   int ret;
-  struct strbuf strbuf;
-  memset (&strbuf, 0, sizeof (strbuf));
 
-  if (!path_parse (path_a, volume, &path)) {
+  if (!path_parse (path_a, volume_a, &path_a1)) {
     cprintf (context, "ERROR bad path '%s'\n", path_a);
     return;
   }
 
-  struct filereader fr;
-  if ((ret = filereader_init (&fr, volume, path)) < 0) {
-    cprintf (context, "ERROR could not initialize filereader for '%s:%s'\n", volume, path);
-    return;
-  }
-
-  size_t chunk_size = 1024;
-  char* buffer = arena_malloc (&arena, chunk_size);
-
-  size_t chunks = fr.file_size / chunk_size;
-  size_t rem = fr.file_size % chunk_size;
-
-  for (size_t chunk = 0; chunk < chunks; chunk++) {
-    if ((ret = filereader_read (&fr, (uint8_t*)buffer, chunk_size)) < 0) {
-      filereader_fini (&fr);
-      cprintf (context, "ERROR filereader failed to read from '%s:%s'\n", volume, path);
-      return;
-    }
-
-    strbuf_append_n (&strbuf, buffer, chunk_size);
-  }
-
-  if (rem > 0) {
-    if ((ret = filereader_read (&fr, (uint8_t*)buffer, rem)) < 0) {
-      filereader_fini (&fr);
-      cprintf (context, "ERROR filereader failed to read from '%s:%s'\n", volume, path);
-      return;
-    }
-
-    strbuf_append_n (&strbuf, buffer, rem);
-  }
-
-  filereader_fini (&fr);
-
-  if (!(path_parse (path_b, volume, &path))) {
+  if (!(path_parse (path_b, volume_b, &path_b1))) {
     cprintf (context, "ERROR bad path '%s'\n", path_b);
     return;
   }
 
-  uint32_t fw_flags = FW_CREATE_FILE | FW_TRUNCATE;
+  struct desc desc;
 
-  struct filewriter fw;
-  if ((ret = filewriter_init (&fw, volume, path, fw_flags)) < 0) {
-    cprintf (context, "ERROR could not initialize filewriter for '%s:%s'\n", volume, path);
+  if ((ret = volume_open (volume_a)) < 0) {
+    cprintf (context, "ERROR could not open volume '%s'\n", volume_a);
     return;
   }
 
-  if (strbuf.count == 0) {
-    filewriter_fini (&fw);
+  if ((ret = describe (path_a1, &desc)) < 0 || desc.type != FS_FILE) {
+    volume_close ();
+    cprintf (context, "ERROR invalid source '%s:%s'\n", volume_a, path_a1);
     return;
   }
 
-  chunk_size = 1024;
-  chunks = (strbuf.count - 1) / chunk_size;
-  rem = (strbuf.count - 1) % chunk_size;
+  size_t file_size = desc.size;
 
-  for (size_t chunk = 0; chunk < chunks; chunk++) {
-    if ((ret = filewriter_write (&fw, (uint8_t*)&strbuf.items[chunk * chunk_size], chunk_size)) <
-        0) {
-      filewriter_fini (&fw);
-      cprintf (context, "ERROR filewriter failed to write to '%s:%s'\n", volume, path);
-      return;
-    }
+  volume_close ();
+
+  if ((ret = volume_open (volume_b)) < 0) {
+    cprintf (context, "ERROR could not open volume '%s'\n", volume_b);
+    return;
   }
 
-  if (rem > 0) {
-    if ((ret = filewriter_write (&fw, (uint8_t*)&strbuf.items[chunks * chunk_size], rem)) < 0) {
-      filewriter_fini (&fw);
-      cprintf (context, "ERROR filewriter failed to write to '%s:%s'\n", volume, path);
-      return;
-    }
+  create_file (path_b1);
+
+  volume_close ();
+
+  size_t chunk_size = 1024;
+  char* buffer = malloc (chunk_size);
+
+  if (buffer == NULL) {
+    return;
   }
 
-  filewriter_fini (&fw);
+  size_t cursor = 0;
+
+  while (cursor < file_size) {
+    size_t to_read = chunk_size;
+
+    if (file_size - cursor < to_read)
+      to_read = file_size - cursor;
+
+    if ((ret = volume_open (volume_a)) < 0) {
+      cprintf (context, "ERROR could not open volume '%s'\n", volume_a);
+      break;
+    }
+
+    if ((ret = read_file (path_a1, cursor, (uint8_t*)buffer, to_read)) < 0) {
+      volume_close ();
+      cprintf (context, "ERROR read '%s:%s'\n", volume_a, path_a1);
+      break;
+    }
+
+    volume_close ();
+
+    if ((ret = volume_open (volume_b)) < 0) {
+      cprintf (context, "ERROR could not open volume '%s'\n", volume_b);
+      break;
+    }
+
+    uint32_t wf_flags = (cursor == 0) ? WF_TRUNCATE : 0;
+
+    if ((ret = write_file (path_b1, cursor, (uint8_t*)buffer, to_read, wf_flags)) < 0) {
+      volume_close ();
+      cprintf (context, "ERROR write '%s:%s'\n", volume_b, path_b1);
+      break;
+    }
+
+    volume_close ();
+
+    cursor += to_read;
+  }
+
+  free (buffer);
 }
 
 static void ls (struct context* context, const char* path_string) {

kernel/device/storage/usbdrv.c

@@ -455,8 +455,8 @@ DEFINE_DEVICE_INIT (usbdrv_init) {
   }
 
   if (ret < 0) {
-    free (usbdrv);
     spin_unlock (&usbdrv->xhci->device->lock, fd);
+    free (usbdrv);
     return false;
   }
 
@@ -465,8 +465,8 @@ DEFINE_DEVICE_INIT (usbdrv_init) {
                                    &usbdrv->sector_count, init->lockflags);
 
   if (ret < 0) {
-    free (usbdrv);
     spin_unlock (&usbdrv->xhci->device->lock, fd);
+    free (usbdrv);
     return false;
   }
 

kernel/device/usb/xhci.c

@@ -233,8 +233,9 @@ static void xhci_event_dispatch (struct xhci* xhci, struct xhci_trb* event, uint
 
     DEBUG ("cmd completion: code=%u,slot=%u\n", cmpl_code, slot_id);
 
-    xhci->last_slot_id = slot_id;
-    xhci->last_cmpl_code = cmpl_code;
+    xhci->event_ring.last_slot_id = slot_id;
+    xhci->event_ring.last_cmpl_code = cmpl_code;
+    atomic_store (&xhci->event_ring.pending, false);
   } break;
   case XHCI_TRB_PORT_STS_CHNG: {
     uint8_t port = ((event->param >> XHCI_PSCETRB_PARAM_PORT_ID) & 0xFF) - 1;
@@ -257,11 +258,30 @@ static void xhci_event_dispatch (struct xhci* xhci, struct xhci_trb* event, uint
     uint8_t slot_id = (event->ctrl >> XHCI_TETRB_CTRL_SLOT_ID) & 0xFF;
     uint8_t endpoint_id = (event->ctrl >> XHCI_TETRB_CTRL_ENDPOINT) & 0x1F;
 
-    /* DEBUG ("transfer completion: code=%u,slot=%u,endpoint_id=%u\n", cmpl_code, slot_id, */
-    /*        endpoint_id); */
+    struct xhci_usb_device* usb_device = NULL;
+    list_find (struct xhci_usb_device, xhci->xhci_usb_devices, usb_device, slot_id, slot_id,
+               usb_devices_link);
 
-    xhci->last_slot_id = slot_id;
-    xhci->last_cmpl_code = cmpl_code;
+    if (usb_device != NULL) {
+      struct xhci_ring* ring = NULL;
+
+      if (endpoint_id == 1) {
+        ring = &usb_device->endpoint0_ring;
+      } else {
+        for (size_t ep = 0; ep < usb_device->endpoints_count; ep++) {
+          if (xhci_get_device_ctx_idx (&usb_device->endpoints[ep].desc) == endpoint_id) {
+            ring = &usb_device->endpoints[ep].transfer_ring;
+            break;
+          }
+        }
+      }
+
+      if (ring != NULL) {
+        ring->last_cmpl_code = cmpl_code;
+        ring->last_slot_id = slot_id;
+        atomic_store (&ring->pending, false);
+      }
+    }
   } break;
   default:
     DEBUG ("Unhandled event type %u at %u\n", type, xhci->event_ring.idx);
@@ -300,8 +320,6 @@ static void xhci_poll_events (struct xhci* xhci) {
 
     xhci_write32 (ir_base, XHCI_ERDP, (uint32_t)dequeue_ptr | (1 << 3));
     xhci_write32 (ir_base, XHCI_ERDP + 4, (uint32_t)(dequeue_ptr >> 32));
-
-    atomic_store (&xhci->pending, false);
   }
 }
 
@@ -397,7 +415,7 @@ static bool xhci_endpoint0_ctrl_in (struct xhci* xhci, struct xhci_usb_device* u
   trb.ctrl = (XHCI_TRB_STATUS_STAGE << XHCI_STSTRB_CTRL_TRB_TYPE) | (1 << XHCI_STSTRB_CTRL_IOC);
   xhci_ring_put_trb (&usb_device->endpoint0_ring, trb);
 
-  atomic_store (&xhci->pending, true);
+  atomic_store (&usb_device->endpoint0_ring.pending, true);
 
   int timeout = 100;
 
@@ -405,7 +423,7 @@ static bool xhci_endpoint0_ctrl_in (struct xhci* xhci, struct xhci_usb_device* u
 
   xhci_write32 (xhci->xhci_doorbell_base, usb_device->slot_id * 4, 1);
 
-  while (atomic_load (&xhci->pending) && --timeout > 0)
+  while (atomic_load (&usb_device->endpoint0_ring.pending) && --timeout > 0)
     stall_ms (1);
 
   spin_lock (&xhci->device->lock, lockflags);
@@ -415,7 +433,7 @@ static bool xhci_endpoint0_ctrl_in (struct xhci* xhci, struct xhci_usb_device* u
     return false;
   }
 
-  return xhci->last_cmpl_code == 1;
+  return usb_device->endpoint0_ring.last_cmpl_code == 1;
 }
 
 static bool xhci_endpoint0_ctrl_out (struct xhci* xhci, struct xhci_usb_device* usb_device,
@@ -473,7 +491,7 @@ static bool xhci_endpoint0_ctrl_out (struct xhci* xhci, struct xhci_usb_device*
       (1 << XHCI_STSTRB_CTRL_DIR);
   xhci_ring_put_trb (&usb_device->endpoint0_ring, trb);
 
-  atomic_store (&xhci->pending, true);
+  atomic_store (&usb_device->endpoint0_ring.pending, true);
 
   int timeout = 100;
 
@@ -481,7 +499,7 @@ static bool xhci_endpoint0_ctrl_out (struct xhci* xhci, struct xhci_usb_device*
 
   xhci_write32 (xhci->xhci_doorbell_base, usb_device->slot_id * 4, 1);
 
-  while (atomic_load (&xhci->pending) && --timeout > 0)
+  while (atomic_load (&usb_device->endpoint0_ring.pending) && --timeout > 0)
     stall_ms (1);
 
   spin_lock (&xhci->device->lock, lockflags);
@@ -491,7 +509,7 @@ static bool xhci_endpoint0_ctrl_out (struct xhci* xhci, struct xhci_usb_device*
     return false;
   }
 
-  return xhci->last_cmpl_code == 1;
+  return usb_device->endpoint0_ring.last_cmpl_code == 1;
 }
 
 static void xhci_send_cmd (struct xhci* xhci, uint64_t param, uint32_t status, uint32_t ctrl,
@@ -521,7 +539,7 @@ static void xhci_send_cmd (struct xhci* xhci, uint64_t param, uint32_t status, u
 
   xhci->cmd_ring.idx++;
 
-  atomic_store (&xhci->pending, true);
+  atomic_store (&xhci->event_ring.pending, true);
 
   int timeout = 100;
 
@@ -529,7 +547,7 @@ static void xhci_send_cmd (struct xhci* xhci, uint64_t param, uint32_t status, u
 
   xhci_write32 (xhci->xhci_doorbell_base, 0, 0);
 
-  while (atomic_load (&xhci->pending) && --timeout > 0)
+  while (atomic_load (&xhci->event_ring.pending) && --timeout > 0)
     stall_ms (1);
 
   spin_lock (&xhci->device->lock, lockflags);
@@ -670,7 +688,7 @@ static bool xhci_usb_device_setup_init_endpoints (struct xhci* xhci,
 
   pmm_free (input_ctx_phys, 1);
 
-  if (xhci->last_cmpl_code != 1) {
+  if (xhci->event_ring.last_cmpl_code != 1) {
     DEBUG ("Failed to configure endpoints for this device\n");
     return false;
   } else {
@@ -705,15 +723,15 @@ static bool xhci_usb_device_setup_addressing (struct xhci* xhci, struct xhci_usb
 
   uint32_t speed = (portsc >> XHCI_PORTSC_PORTSPEED) & 0x0F;
 
-  xhci->last_cmpl_code = 0;
+  xhci->event_ring.last_cmpl_code = 0;
   xhci_send_cmd (xhci, 0, 0, XHCI_TRB_SLOT_ENAB_CMD << XHCI_GTRB_TRB_TYPE, lockflags);
 
-  if (xhci->last_cmpl_code != 1) {
+  if (xhci->event_ring.last_cmpl_code != 1) {
     DEBUG ("Enable slot failed\n");
     return false;
   }
 
-  usb_device->slot_id = xhci->last_slot_id;
+  usb_device->slot_id = xhci->event_ring.last_slot_id;
 
   uintptr_t out_ctx_phys = pmm_alloc (1);
   void* out_ctx_virt = (void*)(out_ctx_phys + (uintptr_t)hhdm->offset);
@@ -766,13 +784,13 @@ static bool xhci_usb_device_setup_addressing (struct xhci* xhci, struct xhci_usb
     ctx32->endpoints[0].dw[3] = (uint32_t)(usb_device->endpoint0_ring.phys >> 32);
   }
 
-  xhci->last_cmpl_code = 0;
+  xhci->event_ring.last_cmpl_code = 0;
   uint32_t ctrl = (usb_device->slot_id << 24) | (XHCI_TRB_ADDR_DEV_CMD << XHCI_GTRB_TRB_TYPE);
   xhci_send_cmd (xhci, input_ctx_phys, 0, ctrl, lockflags);
 
   pmm_free (input_ctx_phys, 1);
 
-  if (xhci->last_cmpl_code != 1) {
+  if (xhci->event_ring.last_cmpl_code != 1) {
     DEBUG ("Failed to address device. port = %u, slot = %u\n", usb_device->xhci_port->port_value,
            usb_device->slot_id);
     return false;
@@ -1010,7 +1028,7 @@ int xhci_bulk_transfer (struct xhci* xhci, struct xhci_usb_device* usb_device,
                         uint64_t* lockflags) {
   struct xhci_usb_device_endpoint* endpoint = NULL;
 
-  for (size_t ep = 0; ep < XHCI_USB_DEVICE_ENDPOINTS_MAX; ep++) {
+  for (size_t ep = 0; ep < usb_device->endpoints_count; ep++) {
     endpoint = &usb_device->endpoints[ep];
 
     if (endpoint->desc.endpoint_addr == endpoint_addr)
@@ -1027,8 +1045,8 @@ int xhci_bulk_transfer (struct xhci* xhci, struct xhci_usb_device* usb_device,
   if (usb_device->slot_id < 0)
     return -ST_NOT_FOUND;
 
-  xhci->last_cmpl_code = 0;
-  atomic_store (&xhci->pending, true);
+  endpoint->transfer_ring.last_cmpl_code = 0;
+  atomic_store (&endpoint->transfer_ring.pending, true);
 
   uint32_t rem = buffer_size;
   uint32_t current = buffer_phys;
@@ -1065,7 +1083,7 @@ int xhci_bulk_transfer (struct xhci* xhci, struct xhci_usb_device* usb_device,
 
   xhci_write32 (xhci->xhci_doorbell_base, usb_device->slot_id * 4, dci);
 
-  while (atomic_load (&xhci->pending) && --timeout > 0)
+  while (atomic_load (&endpoint->transfer_ring.pending) && --timeout > 0)
     stall_ms (1);
 
   spin_lock (&xhci->device->lock, lockflags);
@@ -1075,7 +1093,7 @@ int xhci_bulk_transfer (struct xhci* xhci, struct xhci_usb_device* usb_device,
     return -ST_TRY_AGAIN;
   }
 
-  return xhci->last_cmpl_code == 1 ? ST_OK : -ST_USB_CTRL_ERROR;
+  return endpoint->transfer_ring.last_cmpl_code == 1 ? ST_OK : -ST_USB_CTRL_ERROR;
 }
 
 DEFINE_DEVICE_OP (xhci_poll_driver) {

kernel/device/usb/xhci.h

@@ -345,6 +345,10 @@ struct xhci_ring {
   uint32_t idx;
   uint32_t size;
   uint8_t cycle_bit;
+
+  atomic_bool pending;
+  uint8_t last_slot_id;
+  uint8_t last_cmpl_code;
 };
 
 struct xhci_usb_device_config {
@@ -422,9 +426,6 @@ struct xhci {
   struct list_node_link* xhci_ports;
   struct list_node_link* xhci_usb_devices;
 
-  atomic_bool pending;
-  uint8_t last_slot_id;
-  uint8_t last_cmpl_code;
   struct list_node_link* port_changes;
 };
 

libu/filereader.c

@@ -23,8 +23,10 @@ int filereader_init (struct filereader* fw, const char* volume, const char* path
     return -FR_DESC_ERROR;
   }
 
-  if (desc.type != FS_FILE)
+  if (desc.type != FS_FILE) {
+    volume_close ();
     return -FR_NOT_FILE;
+  }
 
   fw->file_size = desc.size;
   fw->flags |= FR_OPEN;

libu/filewriter.c

@@ -32,8 +32,10 @@ int filewriter_init (struct filewriter* fw, const char* volume, const char* path
     return -FW_DESC_ERROR;
   }
 
-  if (desc.type != FS_FILE)
+  if (desc.type != FS_FILE) {
+    volume_close ();
     return -FW_NOT_FILE;
+  }
 
   fw->file_size = desc.size;
   fw->flags |= FW_OPEN;

spin/spin.c

@@ -1,6 +1,11 @@
 #include <system.h>
 
 void app_main (void) {
-  for (;;)
+  for (;;) {
     sched ();
+#if defined(__x86_64__)
+    for (int i = 0; i < 1000000; i++)
+      __asm__ volatile ("pause" ::: "memory");
+#endif
+  }
 }

testing/make_test_drive.sh

@@ -5,6 +5,8 @@ set -xe
 rm -f test_drive.img
 
 dd if=/dev/zero of=test_drive.img bs=1M count=128
-parted test_drive.img --script mklabel msdos
-parted test_drive.img --script mkpart primary fat32 1MiB 127MiB
-mkfs.vfat -F 32 --offset 2048 test_drive.img
+parted test_drive.img --script unit s mklabel msdos \
+  mkpart primary fat16 1 20480 \
+  mkpart primary fat32 40481 262143
+mkfs.vfat -F 16 --offset 1 test_drive.img 20480
+mkfs.vfat -F 32 --offset 20481 test_drive.img 241663

usb/usb.c

@@ -32,8 +32,12 @@ static void usb_poll (void) {
     for (;;) {
       device_do (info.key, XUSBCTRL_POLL_DRIVER, NULL, NULL, NULL, NULL);
 
-      for (volatile int i = 0; i < 500; i++)
+      for (volatile int i = 0; i < 1000; i++) {
         sched ();
+#if defined(__x86_64__)
+        __asm__ volatile ("pause" ::: "memory");
+#endif
+      }
     }
   }