On 21.01.22 17:54, Kevin O'Connor wrote:
On Fri, Jan 21, 2022 at 05:41:17PM +0100, Alexander Graf wrote:
On 21.01.22 17:02, Kevin O'Connor wrote:
On Fri, Jan 21, 2022 at 03:28:33PM +0100, Alexander Graf wrote:
On 19.01.22 19:45, Kevin O'Connor wrote:
        if ((ns->block_size * count) > (NVME_PAGE_SIZE * 2)) {
-        /* We need to describe more than 2 pages, rely on PRP List */
-        prp2 = ns->prpl;
+        /* We need to describe more than 2 pages, build PRP List */
+        u32 prpl_len = 0;
+        u64 *prpl = (void*)ns->dma_buffer;
At 15*8=120 bytes of data, do we even need to put the prpl into dma_buffer
or can we just use the stack? Will the array be guaranteed 64bit aligned or
would we need to add an attribute to it?

      u64 prpl[NVME_MAX_PRPL_ENTRIES];

Either way, I don't have strong feelings one way or another. It just seems
more natural to keep the bounce buffer purely as bounce buffer :).
In general it's possible to DMA from the stack (eg,
src/hw/ohci.c:ohci_send_pipe() ).  However, SeaBIOS doesn't guarantee
stack alignment so it would need to be done manually.  Also, I'm not
sure how tight the nvme request completion code is - if it returns
early for some reason it could cause havoc (device dma into random
memory).

Another option might be to make a single global prpl array, but that
would consume the memory even if nvme isn't in use.

FWIW though, I don't see a harm in the single ( u64 *prpl =
(void*)ns->dma_buffer ) line.

Fair, works for me. But then we probably want to also adjust
MAX_PRPL_ENTRIES to match the full page we now have available, no?

I don't think a BIOS disk request can be over 64KiB so I don't think
it matters.  I got the impression the current checks are just "sanity
checks".  I don't see a harm in keeping the sanity check and that size
is as good as any other as far as I know.


It's a bit of both: Sanity checks and code that potentially can be reused outside of the SeaBIOS context. So I would try as hard as possible to not make assumptions that we can only handle max 64kb I/O requests. Plus, if we really wanted to, we could even introduce a new SeaBIOS specific INT to do larger I/O requests.

I won't make a fuss if you want to keep it at 64kb max request size though :).


Alex





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-le...@seabios.org

Reply via email to