On Tue, Jan 24, 2017 at 09:43:41PM +0000, Stecklina, Julian wrote: > On Tue, 2017-01-24 at 12:26 -0500, Kevin O'Connor wrote: > > Thanks. See my comments below. Mostly minor things I noticed. > > Thanks for looking into this. It'll take me a couple of days to post a > new patch, because I'm currently traveling. Expect a new patch next > week.
Thanks. > Btw, some of the issues you pointed out also apply to the AHCI code, > because that was what I was looking at while implementing the NVMe > code. ;) Can you point me to which ones? I see one of the AHCI allocations could potentially be moved out of the fseg, but I don't see anything else. [...] > > > +_Static_assert(sizeof(struct nvme_sqe) == 1U << NVME_SQE_SIZE_LOG, > > > "invalid queue entry size"); > > > +_Static_assert(sizeof(struct nvme_cqe) == 1U << NVME_CQE_SIZE_LOG, > > > "invalid queue entry size"); > > Current SeaBIOS supports being compiled on gcc v3.4 and this > > construct > > isn't supported there. That compiler is pretty old, but for now it's > > going to be easier to just change this in your patch. > > I'll update the patch to remove C11 features and be C89 compatible. Thanks. C99 is fine - it's just _Atomic, _Static_assert, and the one for(u16 i ...). [...] > > Ideally the code would start a thread here: > > run_thread(nvme_controller_setup, pci); > > What's the advantage of that? It runs the controller setup in it's own "thread" - see: https://www.seabios.org/Execution_and_code_flow#Threads It has no real impact on VMs, but it can be significant on real hardware. I prefer to see the drivers use it regardless because the drivers tend to get copied. -Kevin _______________________________________________ SeaBIOS mailing list [email protected] https://www.coreboot.org/mailman/listinfo/seabios
