[SeaBIOS] Re: [PATCH v3 4/4] nvme: Split requests by maximum allowed size

Alexander Graf via SeaBIOS posted 1 patch 2 years, 2 months ago
Failed in applying to current master (apply log)
[SeaBIOS] Re: [PATCH v3 4/4] nvme: Split requests by maximum allowed size
Posted by Alexander Graf via SeaBIOS 2 years, 2 months ago
Hi Matt,

On 18.01.22 18:46, Matt DeVillier wrote:
> and attempting to boot from it:
>
> Booting from Hard Disk...
> sq 0x7aa77bdf next_sqe 0
> sq 0x7aa77bdf commit_sqe 0
> cq 0x7aa77bf1 head 0 -> 1
> sq 0x7aa77bdf advanced to 1
> ns 1 read lba 0+1: 0
> Booting from 0000:7c00
> sq 0x7aa77bdf next_sqe 1
> sq 0x7aa77bdf commit_sqe 1
> cq 0x7aa77bf1 head 1 -> 2
> sq 0x7aa77bdf advanced to 2
> ns 1 read lba 1+1: 0
> sq 0x7aa77bdf next_sqe 2
> sq 0x7aa77bdf commit_sqe 2
> cq 0x7aa77bf1 head 2 -> 3
> sq 0x7aa77bdf advanced to 3
> read io: 00000000 00000000 00010003 40270002


This error means

"PRP Offset Invalid: The Offset field for a PRP entry is invalid. This 
may occur when there is a PRP entry with a non-zero offset after the 
first entry or when the Offset field in any PRP entry is not dword 
aligned (i.e., bits 1:0 are not cleared to 00b)."


which points towards a misaligned PRP entry. The block size of this 
request is 105 sectors (52.5kb), so it must be a PRPL. I don't see any 
obvious code paths that would allow us to ever get into a misaligned 
request here.

That said, while trying to understand what is happening here I did 
stumble over a different weird effect: ns->prp1 gets overwritten with 0 
by some interrupt handler. Could you please try the patch / hack below 
and see if it fixes the problem for you? That way I at least know we're 
hunting the same ghost.

If it doesn't help, I'll send you a debug patch that will give us some 
more information about the PRP list SeaBIOS assembles.


Thanks,

Alex

---


diff --git a/src/hw/nvme.c b/src/hw/nvme.c
index f035fa2..99ad7a8 100644
--- a/src/hw/nvme.c
+++ b/src/hw/nvme.c
@@ -257,7 +257,7 @@ nvme_probe_ns(struct nvme_ctrl *ctrl, u32 ns_idx, u8 
mdts)
          goto free_buffer;
      }

-    struct nvme_namespace *ns = malloc_fseg(sizeof(*ns));
+    struct nvme_namespace *ns = malloc_low(sizeof(*ns));
      if (!ns) {
          warn_noalloc();
          goto free_buffer;



Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrung: Christian Schlaeger, Jonathan Weiss
Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B
Sitz: Berlin
Ust-ID: DE 289 237 879


_______________________________________________
SeaBIOS mailing list -- seabios@seabios.org
To unsubscribe send an email to seabios-leave@seabios.org
[SeaBIOS] Re: [PATCH v3 4/4] nvme: Split requests by maximum allowed size
Posted by Matt DeVillier 2 years, 2 months ago
On Tue, Jan 18, 2022 at 4:06 PM Alexander Graf <graf@amazon.de> wrote:
>
> Hi Matt,

hi Alex,

> On 18.01.22 18:46, Matt DeVillier wrote:
> > and attempting to boot from it:
> >
> > Booting from Hard Disk...
> > sq 0x7aa77bdf next_sqe 0
> > sq 0x7aa77bdf commit_sqe 0
> > cq 0x7aa77bf1 head 0 -> 1
> > sq 0x7aa77bdf advanced to 1
> > ns 1 read lba 0+1: 0
> > Booting from 0000:7c00
> > sq 0x7aa77bdf next_sqe 1
> > sq 0x7aa77bdf commit_sqe 1
> > cq 0x7aa77bf1 head 1 -> 2
> > sq 0x7aa77bdf advanced to 2
> > ns 1 read lba 1+1: 0
> > sq 0x7aa77bdf next_sqe 2
> > sq 0x7aa77bdf commit_sqe 2
> > cq 0x7aa77bf1 head 2 -> 3
> > sq 0x7aa77bdf advanced to 3
> > read io: 00000000 00000000 00010003 40270002
>
>
> This error means
>
> "PRP Offset Invalid: The Offset field for a PRP entry is invalid. This
> may occur when there is a PRP entry with a non-zero offset after the
> first entry or when the Offset field in any PRP entry is not dword
> aligned (i.e., bits 1:0 are not cleared to 00b)."
>
>
> which points towards a misaligned PRP entry. The block size of this
> request is 105 sectors (52.5kb), so it must be a PRPL. I don't see any
> obvious code paths that would allow us to ever get into a misaligned
> request here.
>
> That said, while trying to understand what is happening here I did
> stumble over a different weird effect: ns->prp1 gets overwritten with 0
> by some interrupt handler. Could you please try the patch / hack below
> and see if it fixes the problem for you? That way I at least know we're
> hunting the same ghost.

The hack below does indeed work. Not sure if that's good or bad :)

>
> If it doesn't help, I'll send you a debug patch that will give us some
> more information about the PRP list SeaBIOS assembles.
>
>
> Thanks,
>
> Alex
>
> ---
>
>
> diff --git a/src/hw/nvme.c b/src/hw/nvme.c
> index f035fa2..99ad7a8 100644
> --- a/src/hw/nvme.c
> +++ b/src/hw/nvme.c
> @@ -257,7 +257,7 @@ nvme_probe_ns(struct nvme_ctrl *ctrl, u32 ns_idx, u8
> mdts)
>           goto free_buffer;
>       }
>
> -    struct nvme_namespace *ns = malloc_fseg(sizeof(*ns));
> +    struct nvme_namespace *ns = malloc_low(sizeof(*ns));
>       if (!ns) {
>           warn_noalloc();
>           goto free_buffer;
>
>
>
> Amazon Development Center Germany GmbH
> Krausenstr. 38
> 10117 Berlin
> Geschaeftsfuehrung: Christian Schlaeger, Jonathan Weiss
> Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B
> Sitz: Berlin
> Ust-ID: DE 289 237 879
>
>
_______________________________________________
SeaBIOS mailing list -- seabios@seabios.org
To unsubscribe send an email to seabios-leave@seabios.org